Hi,
I think error messages and error codes serve different purposes. Error
messages provide additional information about the error, but users should
never have to match on a message to handle an error/exception. For this
case, it seems like this is a fatal error so we could get away with just
using an error message. Having said that, InvalidKeyError is not too
specific and I'm OK with that too.
As I said earlier, I do think that we need to change the following
"It is recommended that we upgrade the clients before the broker is
upgraded, so that the clients would be able to understand the new
exception."
This is problematic since we want older clients to work with newer brokers.
That's why I recommended that we only throw this error if the
ProduceRequest is version 3 or higher.
Ismael
P.S. Note that we already send error messages back for the CreateTopics
protocol API (I added that in the previous release).
On Tue, Mar 28, 2017 at 7:22 AM, Mayuresh Gharat <gharatmayures...@gmail.com
> wrote:
> I think, it's OK to do this right now.
> The other KIP will have a wider base to cover as it will include other
> exceptions as well and will take time.
>
> Thanks,
>
> Mayuresh
>
> On Mon, Mar 27, 2017 at 11:20 PM Dong Lin <lindon...@gmail.com> wrote:
>
> > Sorry, I forget that you have mentioned this idea in your previous
> reply. I
> > guess the question is, do we still need this KIP if we can have custom
> > error message specified in the exception via the other KIP?
> >
> >
> > On Mon, Mar 27, 2017 at 11:00 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com> wrote:
> >
> > > Hi Dong,
> > >
> > > I do agree with that as I said before the thought did cross my mind
> and I
> > > am working on getting another KIP ready to have error responses
> returned
> > > back to the client.
> > >
> > > In my opinion, it's OK to add a new error code if it justifies the
> need.
> > As
> > > Ismael, mentioned on the jira, we need a specific non retriable error
> > code
> > > in this case, with specific message, at least until the other KIP is
> > ready.
> > >
> > > Thanks,
> > >
> > > Mayuresh
> > > On Mon, Mar 27, 2017 at 10:55 PM Dong Lin <lindon...@gmail.com> wrote:
> > >
> > > > Hey Mayuresh,
> > > >
> > > > I get that you want to provide a more specific error message to user.
> > > Then
> > > > would it be more useful to have a KIP that allows custom error
> message
> > to
> > > > be returned to client together with the exception in the response?
> For
> > > > example, broker can include in the response
> > PolicyViolationException("key
> > > > can not be null for non-compact topic ${topic}") and client can print
> > > this
> > > > error message in the log. My concern with current KIP that it is not
> > very
> > > > scalable to always have a KIP and class for every new error we may
> see
> > in
> > > > the future. The list of error classes, and the errors that need to be
> > > > caught and handled by the client code, will increase overtime and
> > become
> > > > harder to maintain.
> > > >
> > > > Thanks,
> > > > Dong
> > > >
> > > >
> > > > On Mon, Mar 27, 2017 at 7:20 PM, Mayuresh Gharat <
> > > > gharatmayures...@gmail.com
> > > > > wrote:
> > > >
> > > > > Hi Dong,
> > > > >
> > > > > I had thought about this before and wanted to do similar thing. But
> > as
> > > > was
> > > > > pointed out in the jira ticket, we wanted something more specific
> > than
> > > > > general.
> > > > > The main issue is that we do not propagate server side error
> messages
> > > to
> > > > > clients, right now. I am working on a KIP proposal to propose it.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Mayuresh
> > > > >
> > > > > On Mon, Mar 27, 2017 at 5:55 PM, Dong Lin <lindon...@gmail.com>
> > wrote:
> > > > >
> > > > > > Hey Mayuresh,
> > > > > >
> > > > > > Thanks for the patch. I am wondering if it would be better to
> add a
> > > > more
> > > > > > general error, e.g. InvalidMessageException. The benefit is that
> we
> > > can
> > > > > > reuse this for other message level error instead of adding one
> > > > exception
> > > > > > class for each possible exception in the future. This is similar
> to
> > > the
> > > > > use
> > > > > > of InvalidRequestException. For example, ListOffsetResponse may
> > > return
> > > > > > InvalidRequestException if duplicate partitions are found in the
> > > > > > ListOffsetRequest. We don't return DuplicatedPartitionException
> in
> > > this
> > > > > > case.
> > > > > >
> > > > > > Thanks,
> > > > > > Dong
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Wed, Mar 22, 2017 at 3:07 PM, Mayuresh Gharat <
> > > > > > gharatmayures...@gmail.com
> > > > > > > wrote:
> > > > > >
> > > > > > > Hi All,
> > > > > > >
> > > > > > > We have created KIP-135 to propose that Kafka should return a
> > > > > > non-retriable
> > > > > > > error when the producer produces a message with null key to a
> log
> > > > > > compacted
> > > > > > > topic.
> > > > > > >
> > > > > > > Please find the KIP wiki in the link :
> > > > > > >
> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > 135+%3A+Send+of+null+key+to+a+compacted+topic+should+throw+
> > > > > > > non-retriable+error+back+to+user.
> > > > > > >
> > > > > > >
> > > > > > > We would love to hear your comments and suggestions.
> > > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Mayuresh
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > -Regards,
> > > > > Mayuresh R. Gharat
> > > > > (862) 250-7125
> > > > >
> > > >
> > >
> >
>
