-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
ClientRequest .abort()
and ECONNRESET
#32225
Comments
.abort()
and ECONNRESET
Which test will fail? |
e.g. // test/parallel/test-http-abort-before-end.js
'use strict';
const common = require('../common');
const http = require('http');
const server = http.createServer(common.mustNotCall());
server.listen(0, common.mustCall(() => {
const req = http.request({
method: 'GET',
host: '127.0.0.1',
port: server.address().port
});
req.on('abort', common.mustCall(() => {
server.close();
}));
// This should error?
req.on('error', common.mustNotCall());
req.abort();
req.end();
})); Full list here https://gist.github.com/ronag/379a377e01eb2dfe43c62d54c0220c1a |
I actually think the current behavior is fine and it's the assumption that is incorrect. Aborting before the socket is created should not be considered an error condition. |
@jasnell: What is your opinion on |
Doc deprecates ClientRequest.abort in favor of ClientRequest.destroy. Also improves event order documentation for abort and destroy. Refs: nodejs#32225
Doc deprecates ClientRequest.abort in favor of ClientRequest.destroy. Also improves event order documentation for abort and destroy. Refs: #32225 PR-URL: #32807 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Doc deprecates ClientRequest.abort in favor of ClientRequest.destroy. Also improves event order documentation for abort and destroy. Refs: #32225 PR-URL: #32807 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Doc deprecates ClientRequest.abort in favor of ClientRequest.destroy. Also improves event order documentation for abort and destroy. Refs: #32225 PR-URL: #32807 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Doc deprecates ClientRequest.abort in favor of ClientRequest.destroy. Also improves event order documentation for abort and destroy. Refs: #32225 PR-URL: #32807 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Doc deprecates ClientRequest.abort in favor of ClientRequest.destroy. Also improves event order documentation for abort and destroy. Refs: #32225 PR-URL: #32807 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Continuing the discussion from #32182 (comment)
The current assumption is that always
'error'
or'response'
is emitted on the socket.However, this is not always the case. Consider:
This will fail because the case of calling
req.abort()
before'socket'
will actually result in no error. Whether we get an error or not is a question of timing.The current semantics are:
abort()
beforereq.'socket'
, emitreq.'close'
.abort()
afterreq.'socket'
but beforereq.'response'
, emitreq.'error'
.abort()
afterreq.'response'
, emitreq.'close'
&res.'aborted'
.My main concern (which breaks the above assumption) is the first scenario here. However, I noticed that fixing this will cause lots of other tests to fail.
What can/should we do about this? At a minimum I'd like to clarify the docs.
The text was updated successfully, but these errors were encountered: