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.

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

dgram: append actual errors to ERR_SOCKET_CANNOT_SEND #27865

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ function onListenSuccess() {
function onListenError(err) {
this.removeListener('listening', onListenSuccess);
this[kStateSymbol].queue = undefined;
this.emit('error', new ERR_SOCKET_CANNOT_SEND());
this.emit('error', new ERR_SOCKET_CANNOT_SEND(err));
}


Expand Down
23 changes: 15 additions & 8 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,17 @@ function addNumericalSeparator(val) {
return `${val.slice(0, i)}${res}`;
}

// Append `causedByError` to the `error`.
function addCausedByError(error, message, causedByError) {
let msg = message;
if (causedByError) {
Copy link
Member

Choose a reason for hiding this comment

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

It might actually make sense to remove this if statement. If the cause is e.g., a thrown undefined or false, so be it.

error.cause = causedByError;
if (typeof causedByError.message === 'string')
msg += ` (caused by: ${causedByError.message})`;
sam-github marked this conversation as resolved.
Show resolved Hide resolved
}
return msg;
}

module.exports = {
addCodeToName, // Exported for NghttpError
codes,
Expand Down Expand Up @@ -802,13 +813,7 @@ E('ERR_HTTP2_STATUS_101',
'HTTP status code 101 (Switching Protocols) is forbidden in HTTP/2', Error);
E('ERR_HTTP2_STATUS_INVALID', 'Invalid status code: %s', RangeError);
E('ERR_HTTP2_STREAM_CANCEL', function(error) {
let msg = 'The pending stream has been canceled';
if (error) {
this.cause = error;
if (typeof error.message === 'string')
msg += ` (caused by: ${error.message})`;
}
return msg;
return addCausedByError(this, 'The pending stream has been canceled', error);
}, Error);
E('ERR_HTTP2_STREAM_ERROR', 'Stream closed with error code %s', Error);
E('ERR_HTTP2_STREAM_SELF_DEPENDENCY',
Expand Down Expand Up @@ -1060,7 +1065,9 @@ E('ERR_SOCKET_BAD_TYPE',
E('ERR_SOCKET_BUFFER_SIZE',
'Could not get or set buffer size',
SystemError);
E('ERR_SOCKET_CANNOT_SEND', 'Unable to send data', Error);
E('ERR_SOCKET_CANNOT_SEND', function(error) {
return addCausedByError(this, 'Unable to send data', error);
}, Error);
E('ERR_SOCKET_CLOSED', 'Socket is closed', Error);
E('ERR_SOCKET_DGRAM_IS_CONNECTED', 'Already connected', Error);
E('ERR_SOCKET_DGRAM_NOT_CONNECTED', 'Not connected', Error);
Expand Down
5 changes: 4 additions & 1 deletion test/sequential/test-dgram-implicit-bind-failure.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ process.on('exit', () => {
});

socket.on('error', (err) => {
if (/^Error: fake DNS$/.test(err)) {
const fakeDNSReg = /^Error: fake DNS$/;
if (fakeDNSReg.test(err)) {
// The DNS lookup should fail since it is monkey patched. At that point in
// time, the send queue should be populated with the send() operation. There
// should also be two listeners - this function and the dgram internal one
Expand All @@ -39,6 +40,8 @@ socket.on('error', (err) => {
sendFailures++;
assert.strictEqual(socket[kStateSymbol].queue, undefined);
assert.strictEqual(socket.listenerCount('error'), 1);
assert(err.cause instanceof Error);
assert(fakeDNSReg.test(err.cause));
return;
}

Expand Down