> On Feb. 7, 2014, 4:29 a.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java, > > line 52 > > <https://reviews.apache.org/r/17836/diff/1/?file=479011#file479011line52> > > > > Does it make sense to catch all Throwables so we get meaningful error > > message in all cases? > > Jay Kreps wrote: > No, catching Throwable is a shifty thing to do. Throwable is the union of > application errors (Exception) and unrecoverable errors (OOM etc). Catching > the later and calling it a SchemaException would not be good. > > Neha Narkhede wrote: > Hmm.. how about catching the rest of the unrecoverable errors and at > least logging a meaningful error message for those, before rethrowing those > as is (which is what will happen anyways if the code hits an unrecoverable > error). I just think that hitting these errors due to bugs or whatever and > not having a clue of what happened is kind of annoying.
I think that is not a good idea: http://stackoverflow.com/questions/581878/why-catch-exceptions-in-java-when-you-can-catch-throwables We catch the Exceptions and wrap them for a good error, this should cover virtually all application bugs etc (which might actually be too much, e.g. do we really want to catch NullPointerException). The only thing that catching Throwable does is add to this java.lang.Errors. From the javadoc on Errors: "An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch." > On Feb. 7, 2014, 4:29 a.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java, > > line 53 > > <https://reviews.apache.org/r/17836/diff/1/?file=479011#file479011line53> > > > > This will state the field correctly. One other outstanding issue is > > stating the request type correctly. Would this fix that as well? > > Jay Kreps wrote: > No, as I described in the JIRA the serialization code is just about > reading and writing defined serialized types it doesn't and definitely > shouldn't know that there is such a thing as requests. > > Neha Narkhede wrote: > Understand that we don't have all the fixes now. I guess what I was > saying is that it is very useful to have better error reporting in a new code > base in order to understand bugs. How large is the fix to get the serde to > report the right request type? I think I'm not being clear. The serialization layer is like Avro. It doesn't log. It definitely doesn't log about kafka requests! It doesn't know about kafka requests for the same reason Avro doesn't know about kafka requests! It just knows about serialization--i.e. turning a schema and object into bytes and vice versa. We can definitely improve logging in the Sender when we add that but please please not in the serialization library! - Jay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17836/#review33898 ----------------------------------------------------------- On Feb. 7, 2014, 4:23 a.m., Jay Kreps wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17836/ > ----------------------------------------------------------- > > (Updated Feb. 7, 2014, 4:23 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1241 > https://issues.apache.org/jira/browse/KAFKA-1241 > > > Repository: kafka > > > Description > ------- > > KAFKA-1241 Better error message for underflow on read of struct. > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java > 0c11a04e121904b32673b0a86739e02f03066483 > clients/src/main/java/org/apache/kafka/common/protocol/types/Type.java > 24ac060a82767f913dc926ccfe99d83a596e8cf3 > > Diff: https://reviews.apache.org/r/17836/diff/ > > > Testing > ------- > > > Thanks, > > Jay Kreps > >
