Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: fix NPE when printing records with empty value (MINOR) #3470

Merged
merged 3 commits into from
Oct 3, 2019
Merged

fix: fix NPE when printing records with empty value (MINOR) #3470

merged 3 commits into from
Oct 3, 2019

Conversation

vcrfxia
Copy link
Contributor

@vcrfxia vcrfxia commented Oct 3, 2019

Description

Fixes #3450.

Currently when print topic encounters a JSON record with an empty value, a NullPointerException is thrown:

[2019-10-03 10:17:52,063] ERROR Exception encountered while writing to output stream (io.confluent.ksql.rest.server.resources.streaming.TopicStreamWriter:122)
java.lang.NullPointerException
        at com.fasterxml.jackson.databind.node.ObjectNode.setAll(ObjectNode.java:408)
        at io.confluent.ksql.rest.server.resources.streaming.TopicStream$Format$3$1.print(TopicStream.java:182)
        at io.confluent.ksql.rest.server.resources.streaming.TopicStream$RecordFormatter.lambda$format$1(TopicStream.java:76)
        at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
        at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
        at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
        at java.util.Iterator.forEachRemaining(Iterator.java:116)
        at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
        at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
        at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
        at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
        at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
        at io.confluent.ksql.rest.server.resources.streaming.TopicStream$RecordFormatter.format(TopicStream.java:83)
        at io.confluent.ksql.rest.server.resources.streaming.TopicStreamWriter.write(TopicStreamWriter.java:100)
        at org.glassfish.jersey.message.internal.StreamingOutputProvider.writeTo(StreamingOutputProvider.java:79)
        at org.glassfish.jersey.message.internal.StreamingOutputProvider.writeTo(StreamingOutputProvider.java:61)
        at org.glassfish.jersey.message.internal.WriterInterceptorExecutor$TerminalWriterInterceptor.invokeWriteTo(WriterInterceptorExecutor.java:266)
        at org.glassfish.jersey.message.internal.WriterInterceptorExecutor$TerminalWriterInterceptor.aroundWriteTo(WriterInterceptorExecutor.java:251)
        at org.glassfish.jersey.message.internal.WriterInterceptorExecutor.proceed(WriterInterceptorExecutor.java:163)
        at org.glassfish.jersey.server.internal.JsonWithPaddingInterceptor.aroundWriteTo(JsonWithPaddingInterceptor.java:109)
        at org.glassfish.jersey.message.internal.WriterInterceptorExecutor.proceed(WriterInterceptorExecutor.java:163)
        at org.glassfish.jersey.server.internal.MappableExceptionWrapperInterceptor.aroundWriteTo(MappableExceptionWrapperInterceptor.java:85)
        at org.glassfish.jersey.message.internal.WriterInterceptorExecutor.proceed(WriterInterceptorExecutor.java:163)
        at org.glassfish.jersey.message.internal.MessageBodyFactory.writeTo(MessageBodyFactory.java:1135)
        at org.glassfish.jersey.server.ServerRuntime$Responder.writeResponse(ServerRuntime.java:662)
        at org.glassfish.jersey.server.ServerRuntime$Responder.processResponse(ServerRuntime.java:395)
        at org.glassfish.jersey.server.ServerRuntime$Responder.process(ServerRuntime.java:385)
        at org.glassfish.jersey.server.ServerRuntime$1.run(ServerRuntime.java:280)
        at org.glassfish.jersey.internal.Errors$1.call(Errors.java:272)
        at org.glassfish.jersey.internal.Errors$1.call(Errors.java:268)
        at org.glassfish.jersey.internal.Errors.process(Errors.java:316)
        at org.glassfish.jersey.internal.Errors.process(Errors.java:298)
        at org.glassfish.jersey.internal.Errors.process(Errors.java:268)
        at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:289)
        at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:256)
        at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:703)
        at org.glassfish.jersey.servlet.WebComponent.serviceImpl(WebComponent.java:416)
        at org.glassfish.jersey.servlet.ServletContainer.serviceImpl(ServletContainer.java:409)
        at org.glassfish.jersey.servlet.ServletContainer.doFilter(ServletContainer.java:584)
        at org.glassfish.jersey.servlet.ServletContainer.doFilter(ServletContainer.java:525)
        at org.glassfish.jersey.servlet.ServletContainer.doFilter(ServletContainer.java:462)
        at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1610)
        at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:540)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:255)
        at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1700)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:255)
        at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1345)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:203)
        at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:480)
        at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1667)
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:201)
        at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1247)
        at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:144)
        at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:152)
        at org.eclipse.jetty.server.handler.StatisticsHandler.handle(StatisticsHandler.java:174)
        at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:220)
        at org.eclipse.jetty.server.handler.gzip.GzipHandler.handle(GzipHandler.java:753)
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:132)
        at org.eclipse.jetty.server.Server.handle(Server.java:505)
        at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:370)
        at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:267)
        at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:305)
        at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103)
        at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:117)
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:333)
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:310)
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:168)
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.produce(EatWhatYouKill.java:132)
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:698)
        at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:804)
        at java.lang.Thread.run(Thread.java:748)

This PR fixes the NPE by skipping the record, as we currently do for records with null value.

Testing done

Unit test + manual.

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@vcrfxia vcrfxia requested a review from a team as a code owner October 3, 2019 18:15
@vcrfxia vcrfxia changed the title fix: fix NPE when printing records with empty value fix: fix NPE when printing records with empty value (MINOR) Oct 3, 2019
vcrfxia
vcrfxia commented Oct 3, 2019
View reviewed changes
ksql-rest-app/src/main/java/io/confluent/ksql/rest/server/resources/streaming/TopicStream.java Outdated
@@ -179,7 +180,9 @@ public String print(final ConsumerRecord<String, Bytes> record)

objectNode.put(SchemaUtil.ROWTIME_NAME.name(), record.timestamp());
objectNode.put(SchemaUtil.ROWKEY_NAME.name(), key);
objectNode.setAll((ObjectNode) jsonNode);
if (jsonNode != null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix isn't necessary since the above addition (on line 71) takes care of it, but I figure the redundancy can't hurt since we don't want to hit an NPE if something else we don't know about slips through?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer omitting this fix because jsonNode being null could be a symptom for a different issue (assuming we expect it to be non-null)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

agavra
agavra approved these changes Oct 3, 2019
View reviewed changes
Copy link
Contributor

@agavra agavra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @vcrfxia

ksql-rest-app/src/main/java/io/confluent/ksql/rest/server/resources/streaming/TopicStream.java Outdated
@@ -179,7 +180,9 @@ public String print(final ConsumerRecord<String, Bytes> record)

objectNode.put(SchemaUtil.ROWTIME_NAME.name(), record.timestamp());
objectNode.put(SchemaUtil.ROWKEY_NAME.name(), key);
objectNode.setAll((ObjectNode) jsonNode);
if (jsonNode != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer omitting this fix because jsonNode being null could be a symptom for a different issue (assuming we expect it to be non-null)

@vcrfxia vcrfxia merged commit 47313ff into confluentinc:master Oct 3, 2019
@vcrfxia vcrfxia deleted the empty-value-print-npe branch October 3, 2019 22:50
@agavra agavra mentioned this pull request Oct 17, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agavra agavra agavra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NPE when PRINTing topic
2 participants
@vcrfxia @agavra