Skip to content

Commit 73e6a1c

Browse files
authored
Preserve event error code (#1151)
* Preserve numeric error code and isFatal on event.error EventDispatcher::Flush created the JS error for EVENT_ERROR from the message text only, so the librdkafka error code was lost. LibrdKafkaError then fell back to code -1 (ERR_UNKNOWN) for every event.error, e.g. ERR__ALL_BROKERS_DOWN (-187) and ERR__TRANSPORT (-195). Keep event.err() and event.fatal() in event_t and set code and isFatal on the error object. lib/error.js already prefers a numeric e.code, so no JS change is needed. Adds a test using a minimal fake broker: after librdkafka has learned a real broker and it goes away, event.error must carry ERR__ALL_BROKERS_DOWN. * Bump version to v3.7.0 --------- Co-authored-by: zhangxin511 <3719246+zhangxin511@users.noreply.github.com>
1 parent ce2412b commit 73e6a1c

5 files changed

Lines changed: 138 additions & 3 deletions

File tree

‎package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "node-rdkafka",
3-
"version": "v3.6.1",
3+
"version": "v3.7.0",
44
"description": "Node.js bindings for librdkafka",
55
"librdkafka": "2.12.0",
66
"main": "lib/index.js",

‎src/callbacks.cc‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,16 @@ void Dispatcher::RemoveCallback(const v8::Local<v8::Function> &cb) {
122122
event_t::event_t(const RdKafka::Event &event) {
123123
message = "";
124124
fac = "";
125+
err = RdKafka::ERR_NO_ERROR;
126+
fatal = false;
125127

126128
type = event.type();
127129

128130
switch (type = event.type()) {
129131
case RdKafka::Event::EVENT_ERROR:
130132
message = RdKafka::err2str(event.err());
133+
err = event.err();
134+
fatal = event.fatal();
131135
break;
132136
case RdKafka::Event::EVENT_STATS:
133137
message = event.str();
@@ -202,6 +206,13 @@ void EventDispatcher::Flush() {
202206
argv[0] = Nan::New("error").ToLocalChecked();
203207
argv[1] = Nan::Error(_events[i].message.c_str());
204208

209+
// Keep the numeric librdkafka error code. Without it LibrdKafkaError
210+
// falls back to code -1 (ERR_UNKNOWN) for every event error.
211+
Nan::Set(argv[1].As<v8::Object>(), Nan::New("code").ToLocalChecked(),
212+
Nan::New<Number>(static_cast<int>(_events[i].err)));
213+
Nan::Set(argv[1].As<v8::Object>(), Nan::New("isFatal").ToLocalChecked(),
214+
Nan::New<v8::Boolean>(_events[i].fatal));
215+
205216
// if (event->err() == RdKafka::ERR__ALL_BROKERS_DOWN). Stop running
206217
// This may be better suited to the node side of things
207218
break;

‎src/callbacks.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ struct event_t {
5656
RdKafka::Event::Type type;
5757
std::string message;
5858

59+
// Only set for EVENT_ERROR
60+
RdKafka::ErrorCode err;
61+
bool fatal;
62+
5963
RdKafka::Event::Severity severity;
6064
std::string fac;
6165

‎test/event-error.spec.js‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
/*
2+
* node-rdkafka - Node.js wrapper for RdKafka C/C++ library
3+
*
4+
* Copyright (c) 2016 Blizzard Entertainment
5+
*
6+
* This software may be modified and distributed under the terms
7+
* of the MIT license. See the LICENSE.txt file for details.
8+
*/
9+
10+
var net = require('net');
11+
var Producer = require('../lib/producer');
12+
var LibrdKafkaError = require('../lib/error');
13+
var t = require('assert');
14+
15+
// Minimal fake broker speaking only Metadata v0. It answers one request with
16+
// itself as the only broker, so librdkafka learns a real (non-bootstrap)
17+
// broker, and is then shut down. librdkafka raises ERR__ALL_BROKERS_DOWN
18+
// only in that situation, as it does when a real cluster becomes unreachable.
19+
function startFakeBroker(cb) {
20+
var sockets = [];
21+
var server = net.createServer(function(socket) {
22+
sockets.push(socket);
23+
socket.on('error', function() {});
24+
var pending = Buffer.alloc(0);
25+
socket.on('data', function(data) {
26+
pending = Buffer.concat([pending, data]);
27+
// size(4) apiKey(2) apiVersion(2) correlationId(4) ...
28+
// librdkafka may pipeline several requests in one chunk.
29+
while (pending.length >= 4 && pending.length >= 4 + pending.readInt32BE(0)) {
30+
var frame = pending.slice(0, 4 + pending.readInt32BE(0));
31+
pending = pending.slice(frame.length);
32+
if (frame.readInt16BE(4) !== 3) {
33+
continue;
34+
}
35+
var host = Buffer.from('127.0.0.1');
36+
var body = Buffer.alloc(4 + 4 + 4 + 2 + host.length + 4 + 4);
37+
var o = 0;
38+
o = body.writeInt32BE(frame.readInt32BE(8), o); // correlation id
39+
o = body.writeInt32BE(1, o); // brokers: 1
40+
o = body.writeInt32BE(0, o); // node id
41+
o = body.writeInt16BE(host.length, o);
42+
o += host.copy(body, o);
43+
o = body.writeInt32BE(server.address().port, o);
44+
body.writeInt32BE(0, o); // topics: 0
45+
var size = Buffer.alloc(4);
46+
size.writeInt32BE(body.length, 0);
47+
socket.write(Buffer.concat([size, body]));
48+
}
49+
});
50+
});
51+
52+
server.listen(0, '127.0.0.1', function() {
53+
cb(server, function stop() {
54+
sockets.forEach(function(s) { s.destroy(); });
55+
server.close();
56+
});
57+
});
58+
}
59+
60+
module.exports = {
61+
'event.error': {
62+
'keeps the numeric librdkafka error code': function(cb) {
63+
startFakeBroker(function(server, stop) {
64+
var client = new Producer({
65+
'client.id': 'kafka-mocha-event-error',
66+
'metadata.broker.list': '127.0.0.1:' + server.address().port,
67+
'api.version.request': false,
68+
'broker.version.fallback': '0.9.0',
69+
'reconnect.backoff.max.ms': 100,
70+
'socket.keepalive.enable': true
71+
}, {});
72+
73+
var done = false;
74+
var timer;
75+
var finish = function(err) {
76+
if (done) {
77+
return;
78+
}
79+
done = true;
80+
clearTimeout(timer);
81+
stop();
82+
client.disconnect(function() {
83+
cb(err);
84+
});
85+
};
86+
87+
timer = setTimeout(function() {
88+
finish(new Error('Timed out waiting for ERR__ALL_BROKERS_DOWN'));
89+
}, 20000);
90+
91+
client.on('event.error', function(err) {
92+
if (err.code === LibrdKafkaError.codes.ERR__ALL_BROKERS_DOWN) {
93+
try {
94+
t.strictEqual(err.errno, err.code);
95+
t.strictEqual(err.origin, 'local');
96+
t.strictEqual(typeof err.isFatal, 'boolean');
97+
} catch (e) {
98+
return finish(e);
99+
}
100+
return finish();
101+
}
102+
// Other errors (e.g. ERR__TRANSPORT) can arrive first. They must
103+
// carry their real code and not fall back to -1.
104+
if (err.code === LibrdKafkaError.codes.ERR_UNKNOWN) {
105+
finish(new Error('event.error lost its code: ' + err.message));
106+
}
107+
});
108+
109+
client.connect({}, function(err) {
110+
if (err) {
111+
return finish(err);
112+
}
113+
// connected and the broker is known; now make it unreachable
114+
stop();
115+
});
116+
client.setPollInterval(50);
117+
});
118+
}
119+
}
120+
};

0 commit comments

Comments
 (0)