nodejs-github-bot · GitHub

@nodejs-github-bot added the lib / src

Issues and PRs related to general changes in the lib or src directory.

label

Apr 23, 2018

@ronag ronag changed the title Http client boolean aborted http: client aborted should be boolean.

Apr 23, 2018

@vsemozhetbyt vsemozhetbyt added http

Issues or PRs related to the http subsystem.

http2

Issues or PRs related to the http2 subsystem.

labels

Apr 23, 2018

vsemozhetbyt

@ronag

@ronag ronag mentioned this pull request

Apr 23, 2018

Closed

3 tasks

mscdex

@ronag

@lpinca lpinca added the semver-major

PRs that contain breaking changes and should be released in the next major version.

label

Apr 26, 2018

@ronag

lpinca

@ronag

@lpinca lpinca removed http2

Issues or PRs related to the http2 subsystem.

lib / src

Issues and PRs related to general changes in the lib or src directory.

labels

Apr 26, 2018

@ronag

addaleax

mcollina

@ronag

@ronag

vsemozhetbyt

vsemozhetbyt pushed a commit that referenced this pull request

May 14, 2018
PR-URL: #20230
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

mastermatt added a commit to mastermatt/nock that referenced this pull request

Mar 24, 2020
Nocks behavior around events and errors relating to aborting and ending requests has not lined up
with Node for while. I dug into an investigation of how it differs, if there were git commits/pull-requests explaining why they differ,
and trying to answer the question of should Nock change to line up better.
The single largest deviation were the errors emitted around aborting or already aborted requests.
Emitting an error on abort was added to fix issue nock#439.
The conversation talks about how Node emits an error post abort.
> ... in practice the `http` module does emit an `ECONNRESET` error after aborting. ... - Jan 7, 2016
However, this is only true between the 'socket' and 'response' events.
Otherwise calling abort does not emit an error of any kind.
Determining exactly what Nock should do is a bit of moving target.
As Node's internals of HTTP, Net, Stream, and micro task timing have changed a lot since
iojs/v4, when Nock added these errors, and v13, current at this time.
My conclusion is that none of Nock's deviations from Node's _documented_ behavior
are user features that should be maintained. If anything, bringing Nock closer to Node
better fulfills original requests.
nock#282
The entire `test_abort.js` file was scraped and rewritten by writing assertions against Node behavior, without Nock first.
Then adding Nock into each test to ensure events and errors were correct.
BREAKING CHANGE:
Calling `request.abort()`:
- Use to always emit a 'socket hang up' error. Now only emits the error if `abort` is called between the 'socket' and 'response' events.
- The emitted 'abort' event now happens on `nextTick`.
- The socket is now only destroyed if the 'socket' event has fired, and now emits a 'close' event on `nextTick` that propagates though the request object.
- `request.aborted` attribute is set to `true` instead of a timestamp. Changed in Node v11.0 nodejs/node#20230
Calling `write`, `end`, or `flushHeaders` on an aborted request no longer emits an error.
However, writing to a request that is already finished (ended) will emit a 'write after end' error.
Playback of a mocked responses will now never happen until the 'socket' event is emitted.
The 'socket' event is still artificially set to emit on `nextTick` when a ClientRequest is created.
This means in the following code the Scope will never be done because at least one tick needs
to happen before any matched Interceptor is considered consumed.
```js
const scope = nock(...).get('/').reply()
const req = http.get(...)
scope.done()
```

Merged

mastermatt added a commit to mastermatt/nock that referenced this pull request

Mar 29, 2020
Nocks behavior around events and errors relating to aborting and ending requests has not lined up
with Node for while. I dug into an investigation of how it differs, if there were git commits/pull-requests explaining why they differ,
and trying to answer the question of should Nock change to line up better.
The single largest deviation were the errors emitted around aborting or already aborted requests.
Emitting an error on abort was added to fix issue nock#439.
The conversation talks about how Node emits an error post abort.
> ... in practice the `http` module does emit an `ECONNRESET` error after aborting. ... - Jan 7, 2016
However, this is only true between the 'socket' and 'response' events.
Otherwise calling abort does not emit an error of any kind.
Determining exactly what Nock should do is a bit of moving target.
As Node's internals of HTTP, Net, Stream, and micro task timing have changed a lot since
iojs/v4, when Nock added these errors, and v13, current at this time.
My conclusion is that none of Nock's deviations from Node's _documented_ behavior
are user features that should be maintained. If anything, bringing Nock closer to Node
better fulfills original requests.
nock#282
The entire `test_abort.js` file was scraped and rewritten by writing assertions against Node behavior, without Nock first.
Then adding Nock into each test to ensure events and errors were correct.
BREAKING CHANGE:
Calling `request.abort()`:
- Use to always emit a 'socket hang up' error. Now only emits the error if `abort` is called between the 'socket' and 'response' events.
- The emitted 'abort' event now happens on `nextTick`.
- The socket is now only destroyed if the 'socket' event has fired, and now emits a 'close' event on `nextTick` that propagates though the request object.
- `request.aborted` attribute is set to `true` instead of a timestamp. Changed in Node v11.0 nodejs/node#20230
Calling `write`, `end`, or `flushHeaders` on an aborted request no longer emits an error.
However, writing to a request that is already finished (ended) will emit a 'write after end' error.
Playback of a mocked responses will now never happen until the 'socket' event is emitted.
The 'socket' event is still artificially set to emit on `nextTick` when a ClientRequest is created.
This means in the following code the Scope will never be done because at least one tick needs
to happen before any matched Interceptor is considered consumed.
```js
const scope = nock(...).get('/').reply()
const req = http.get(...)
scope.done()
```

mastermatt added a commit to nock/nock that referenced this pull request

Apr 4, 2020
Nocks behavior around events and errors relating to aborting and ending requests has not lined up
with Node for while. I dug into an investigation of how it differs, if there were git commits/pull-requests explaining why they differ,
and trying to answer the question of should Nock change to line up better.
The single largest deviation were the errors emitted around aborting or already aborted requests.
Emitting an error on abort was added to fix issue #439.
The conversation talks about how Node emits an error post abort.
> ... in practice the `http` module does emit an `ECONNRESET` error after aborting. ... - Jan 7, 2016
However, this is only true between the 'socket' and 'response' events.
Otherwise calling abort does not emit an error of any kind.
Determining exactly what Nock should do is a bit of moving target.
As Node's internals of HTTP, Net, Stream, and micro task timing have changed a lot since
iojs/v4, when Nock added these errors, and v13, current at this time.
My conclusion is that none of Nock's deviations from Node's _documented_ behavior
are user features that should be maintained. If anything, bringing Nock closer to Node
better fulfills original requests.
#282
The entire `test_abort.js` file was scraped and rewritten by writing assertions against Node behavior, without Nock first.
Then adding Nock into each test to ensure events and errors were correct.
BREAKING CHANGE:
Calling `request.abort()`:
- Use to always emit a 'socket hang up' error. Now only emits the error if `abort` is called between the 'socket' and 'response' events.
- The emitted 'abort' event now happens on `nextTick`.
- The socket is now only destroyed if the 'socket' event has fired, and now emits a 'close' event on `nextTick` that propagates though the request object.
- `request.aborted` attribute is set to `true` instead of a timestamp. Changed in Node v11.0 nodejs/node#20230
Calling `write`, `end`, or `flushHeaders` on an aborted request no longer emits an error.
However, writing to a request that is already finished (ended) will emit a 'write after end' error.
Playback of a mocked responses will now never happen until the 'socket' event is emitted.
The 'socket' event is still artificially set to emit on `nextTick` when a ClientRequest is created.
This means in the following code the Scope will never be done because at least one tick needs
to happen before any matched Interceptor is considered consumed.
```js
const scope = nock(...).get('/').reply()
const req = http.get(...)
scope.done()
```

juninmd pushed a commit to juninmd/nock that referenced this pull request

Mar 21, 2026
…k#1960)
Nocks behavior around events and errors relating to aborting and ending requests has not lined up
with Node for while. I dug into an investigation of how it differs, if there were git commits/pull-requests explaining why they differ,
and trying to answer the question of should Nock change to line up better.
The single largest deviation were the errors emitted around aborting or already aborted requests.
Emitting an error on abort was added to fix issue nock#439.
The conversation talks about how Node emits an error post abort.
> ... in practice the `http` module does emit an `ECONNRESET` error after aborting. ... - Jan 7, 2016
However, this is only true between the 'socket' and 'response' events.
Otherwise calling abort does not emit an error of any kind.
Determining exactly what Nock should do is a bit of moving target.
As Node's internals of HTTP, Net, Stream, and micro task timing have changed a lot since
iojs/v4, when Nock added these errors, and v13, current at this time.
My conclusion is that none of Nock's deviations from Node's _documented_ behavior
are user features that should be maintained. If anything, bringing Nock closer to Node
better fulfills original requests.
nock#282
The entire `test_abort.js` file was scraped and rewritten by writing assertions against Node behavior, without Nock first.
Then adding Nock into each test to ensure events and errors were correct.
BREAKING CHANGE:
Calling `request.abort()`:
- Use to always emit a 'socket hang up' error. Now only emits the error if `abort` is called between the 'socket' and 'response' events.
- The emitted 'abort' event now happens on `nextTick`.
- The socket is now only destroyed if the 'socket' event has fired, and now emits a 'close' event on `nextTick` that propagates though the request object.
- `request.aborted` attribute is set to `true` instead of a timestamp. Changed in Node v11.0 nodejs/node#20230
Calling `write`, `end`, or `flushHeaders` on an aborted request no longer emits an error.
However, writing to a request that is already finished (ended) will emit a 'write after end' error.
Playback of a mocked responses will now never happen until the 'socket' event is emitted.
The 'socket' event is still artificially set to emit on `nextTick` when a ClientRequest is created.
This means in the following code the Scope will never be done because at least one tick needs
to happen before any matched Interceptor is considered consumed.
```js
const scope = nock(...).get('/').reply()
const req = http.get(...)
scope.done()
```

Read the original on github.com ↗