Node-rdkafka: LibrdKafkaError loses librdkafka error code passed when constructed from existing error, reset to -1

[JaapRood]

Trying to improve the fault tolerance of own of our deployments running a KafkaConsumer, I've been investigating automatically retrying making the initial connection to the Kafka cluster. Only wanting to do so on a strict set of temporary errors (like Broker transport failure), I'm relying on the error.code produced by client.connect. However, through the creation of a LibrdKafkaError, the error code is reset to -1: unknown.

https://github.com/Blizzard/node-rdkafka/blob/2cd56d88f8b7fd13b0d59844f8bbe908353a0cee/lib/client.js#L185-L189

Inspecting the error at that point in the code yields:

{ message: 'Local: Broker transport failure', code: -195 }

It's then reset in the construction of the LibrdKafkaError:

https://github.com/Blizzard/node-rdkafka/blob/2cd56d88f8b7fd13b0d59844f8bbe908353a0cee/lib/error.js#L227-L255

From what I can understand resetting the error code to -1 comes from the assumption that this error could have been any type of Error thrown in the rest of Node, not necessarily an error coming from librdkafka.

What I'm wondering if there's anyway we can prevent this, so we can handle these types of errors with more granularity.

[JaapRood]

Some extra background around implementing a retry on connection: librdafka is great at handling almost every error case once a connection has been made, leaving very little to worry about. However, any errors upon creating a connection (or subscription, assignment, etc) are thrown.

In development this makes perfect sense to me: odds are you've misconfigured something, like not having spun up a local Kafka cluster. That said, we ran into an issue our production environment a short while back where, due to network problems, the entire cluster couldn't be reached. As we deployed a new version and services got restarted, we started seeing our processes crashing (exiting with non-zero status) repeatedly.

You could argue that this is might be the behaviour you'd want, letting the process manager deal with the retrying. While that's fine for background tasks, we also run consumers as part of our web servers, replicating a topic we use as a changelog to a local store, to be used in the serving of requests. We'd at least want to be able to send proper 503 requests for the affected endpoints, rather than the whole service being down (e.g. a fault tolerant system!).

[JaapRood]

@webmakersteve I posted this as an issue rather than as a PR as I see error handling and the resulting fault tolerance as something I don't want to disturb with my limited knowledge of the rest of the codebase. That said, with a bit of direction on the right approach to deal with this, I'm more than happy to put in the work to develop a fix.

The most naive thing I can think of is propagating the error code if it's available, like:

   } else { 
     this.origin = origin; 
     this.message = msg;
     this.code = typeof e.code === "number" ? e.code : -1; 
     this.errno = this.code; 
     this.stack = e.stack; 
   }

[JaapRood]

I've found that this same behaviour occurs after the sending of a message times out. While the error comes in with the correct -185 code, the error in the delivery report has a code of -1.

[webmakersteve]

I'd be happy to merge your suggested fix if you submit it as a PR. It looks like it should handle this case fine. Otherwise, I can do it myself if you'd prefer, but would like to give you credit :)

[JaapRood]

@webmakersteve should be all ready to go, definitely helping me get the retry logic sorted out. I'd be happy to contribute that retry logic as well, if you're at all interested. I thought that might be appropriate for at least the KafkaConsumerStream and ProducerStream, as their API's are already node-specific and have managed connections, rather than just a binding to librdkafka. Let me know your thoughts.

[webmakersteve]

I'd be happy to consider adding additional retry logic in the streams if it makes sense. If you want to either submit a PR for my review that we can discuss, or just send me a preliminary description of how you're doing it, I would definitely take a look :)

[ankon]

I'm not sure whether to file a new issue, or piggyback things onto this one: We also got hit by this problem, and noticed that not just the error code vanishes, but also that the logic to parse the message changes the message: The initial error from rdkafka 'Local: Broker transport failure' would change into 'broker transport failure' when processed by LibrdkafkaError's constructor.

I see a couple of things here:

1. Somehow avoid the parsing magic when the error in question is already a librdkafka error instance completely
2. Figure out _why_ there are paths through the code that wrap an error multiple times

Any advise how to proceed best?

[webmakersteve]

I would like to map librdkafka errors to node Error objects with code and message exactly as specified from librdkafka. Additionally, I do not want to catch any errors in the lower level implementation methods and surface them directly to the client - and get rid of the error event. Some of that stuff should transition to model that it is not synonymous with a standard error event in a node EventEmitter.

Does that answer your question at all?

[JaapRood]

and get rid of the error event.

@webmakersteve I was wondering which error event you mean exactly? One that already is propagated through some other mechanism?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.