Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

http: doc deprecate abort and improve docs #32807

Closed
wants to merge 8 commits into from
Closed

http: doc deprecate abort and improve docs #32807

wants to merge 8 commits into from

Conversation

ronag
Copy link
Member

@ronag ronag commented Apr 13, 2020 •
edited
Loading

Doc deprecates ClientRequest.abort in favor of
ClientRequest.destroy. Also improves event order
documentation for abort and destroy.

Refs: #32225

Please note that destroy() and abort() slightly differ in the
case of ... called before a socket is assigned .... I'm not sure where we landed in regards to whether or not to address this difference in #32225. This PR is based on the current (master) state of things.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 13, 2020
ronag
ronag commented Apr 13, 2020
View reviewed changes
doc/api/http.md

* (`req.abort()` called here)
* `'abort'`
* `'close'`
Copy link
Member Author

@ronag ronag Apr 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This req.abort() variant was previously not documented.

ronag
ronag commented Apr 13, 2020
View reviewed changes
doc/api/http.md
* (`req.destroy()` called here)
* `'error'` with an error with message `'Error: socket hang up'` and code
`'ECONNRESET'`
* `'close'`
Copy link
Member Author

Choose a reason for hiding this comment

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

req.destroy() behaves different than req.abort() in this case

@ronag
Copy link
Member Author

ronag commented Apr 13, 2020

@nodejs/http

@ronag
http: doc deprecate abort and improve docs
69a8af0 
Doc deprecates ClientRequest.abort in favor of
ClientRequest.destroy. Also improves event order
documentation for abort and destroy.

Refs: nodejs#32225
@ronag ronag force-pushed the http-docs-deprecate-abort branch from e291307 to 69a8af0 Compare April 13, 2020 09:08
@ronag
Copy link
Member Author

ronag commented Apr 13, 2020 •
edited
Loading

@himself65: What does the 👀 mean? 😄

@himself65
Copy link
Member

@himself65: What does the 👀 mean? 😄

I noticed and still reviewing.😀

mcollina
mcollina approved these changes Apr 13, 2020
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina requested review from mcollina and a team April 13, 2020 10:23
@ronag
Copy link
Member Author

ronag commented Apr 13, 2020

Does this require a semver-major label?

@ronag ronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 13, 2020
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/30679/

@mcollina mcollina added the semver-major PRs that contain breaking changes and should be released in the next major version. label Apr 13, 2020
@mcollina
Copy link
Member

Yes

@ronag ronag force-pushed the http-docs-deprecate-abort branch from 7dfe4c7 to 78553bd Compare April 13, 2020 11:06
@ronag ronag force-pushed the http-docs-deprecate-abort branch from 78553bd to 083ad2e Compare April 13, 2020 11:08
@ronag
Copy link
Member Author

ronag commented Apr 13, 2020

Added missing docs for request.destroyed and request.destroy(). If this doesn't land on 14.x I will open a backport PR without the deprecation changes.

addaleax
addaleax approved these changes Apr 13, 2020
View reviewed changes
ronag and others added 2 commits April 13, 2020 15:16
@ronag @addaleax
Apply suggestions from code review
08e542a 
Co-Authored-By: Anna Henningsen <github@addaleax.net>
@ronag
fixup: further details
72b2694
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/30684/

@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 13, 2020
@ronag
Copy link
Member Author

ronag commented Apr 26, 2020

Landed in 5ee1e31

@ronag ronag closed this Apr 26, 2020
ronag added a commit that referenced this pull request Apr 26, 2020
@ronag
http: doc deprecate abort and improve docs
5ee1e31 
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>
BethGriggs pushed a commit that referenced this pull request Apr 27, 2020
@ronag @BethGriggs
http: doc deprecate abort and improve docs
ce1af77 
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>
@BethGriggs BethGriggs mentioned this pull request Apr 27, 2020
Merged
@richardlau richardlau mentioned this pull request Apr 27, 2020
Closed
3 tasks
addaleax pushed a commit that referenced this pull request Apr 27, 2020
@richardlau @addaleax
doc: assign missing deprecation code
76ba950 
Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: #33109
Refs: #32807
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
BethGriggs pushed a commit that referenced this pull request Apr 28, 2020
@richardlau @BethGriggs
doc: assign missing deprecation code
00c3ed9 
Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: #33109
Refs: #32807
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
@ronag @BridgeAR
http: doc deprecate abort and improve docs
4d9e69d 
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>
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
@richardlau @BridgeAR
doc: assign missing deprecation code
fc9191a 
Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: #33109
Refs: #32807
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
@ronag @BridgeAR
http: doc deprecate abort and improve docs
9d18408 
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>
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
@richardlau @BridgeAR
doc: assign missing deprecation code
386d158 
Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: #33109
Refs: #32807
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
@BridgeAR BridgeAR mentioned this pull request Apr 28, 2020
Merged
BethGriggs pushed a commit that referenced this pull request Apr 28, 2020
@ronag @BethGriggs
http: doc deprecate abort and improve docs
31c797c 
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>
BethGriggs pushed a commit that referenced this pull request Apr 28, 2020
@richardlau @BethGriggs
doc: assign missing deprecation code
fa5848e 
Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: #33109
Refs: #32807
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
BethGriggs added a commit that referenced this pull request Apr 28, 2020
@BethGriggs
2020-04-28, Version 14.1.0 (Current)
9a3476e 
Notable changes:

- deps: upgrade openssl sources to 1.1.1g (Hassaan Pasha)
  [#32971](#32971)
- doc: add juanarbol as collaborator (Juan José Arboleda)
  [#32906](#32906)
- http: doc deprecate abort and improve docs (Robert Nagy)
  [#32807](#32807)
- module: do not warn when accessing `__esModule` of unfinished exports
  (Anna Henningsen) [#33048](#33048)
- n-api: detect deadlocks in thread-safe function (Gabriel Schulhof)
  [#32860](#32860)
- src: deprecate embedder APIs with replacements (Anna Henningsen)
  [#32858](#32858)
- stream:
  - don't emit end after close (Robert Nagy)
    [#33076](#33076)
  - don't wait for close on legacy streams (Robert Nagy)
    [#33058](#33058)
  - pipeline should only destroy un-finished streams (Robert Nagy)
    [#32968](#32968)

PR-URL: #33103
BethGriggs pushed a commit that referenced this pull request Apr 28, 2020
@richardlau @BethGriggs
doc: assign missing deprecation code
343c6ac 
Signed-off-by: Richard Lau <riclau@uk.ibm.com>

PR-URL: #33109
Refs: #32807
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
BethGriggs added a commit that referenced this pull request Apr 28, 2020
@BethGriggs
2020-04-29, Version 14.1.0 (Current)
9706e19 
Notable changes:

- deps: upgrade openssl sources to 1.1.1g (Hassaan Pasha)
  [#32971](#32971)
- doc: add juanarbol as collaborator (Juan José Arboleda)
  [#32906](#32906)
- http: doc deprecate abort and improve docs (Robert Nagy)
  [#32807](#32807)
- module: do not warn when accessing `__esModule` of unfinished exports
  (Anna Henningsen) [#33048](#33048)
- n-api: detect deadlocks in thread-safe function (Gabriel Schulhof)
  [#32860](#32860)
- src: deprecate embedder APIs with replacements (Anna Henningsen)
  [#32858](#32858)
- stream:
  - don't emit end after close (Robert Nagy)
    [#33076](#33076)
  - don't wait for close on legacy streams (Robert Nagy)
    [#33058](#33058)
  - pipeline should only destroy un-finished streams (Robert Nagy)
    [#32968](#32968)
- vm: add importModuleDynamically option to compileFunction (Gus Caplan)
  [#32985](#32985)

PR-URL: #33103
dr-js added a commit to dr-js/dr-js that referenced this pull request Apr 29, 2020
@dr-js
@dr-js/core@0.3.0-dev.10
3394803 
notable change:
- node: use `request.destroy()|request.destroyed` instead of `request.abort()|request.aborted` follow: nodejs/node#32807
- package update
dr-js added a commit to dr-js/dr-node that referenced this pull request Apr 29, 2020
@dr-js
@dr-js/node@0.3.0-dev.5
d8f2c5f 
notable change:
- node: use `request.destroy()|request.destroyed` instead of `request.abort()|request.aborted` follow: nodejs/node#32807
- add: `extractTgzOrTarAsync` to `module/Software/7z`
- script explorer compress code
- package update
BethGriggs added a commit that referenced this pull request Apr 29, 2020
@BethGriggs
2020-04-29, Version 14.1.0 (Current)
9ffd8e7 
Notable changes:

- deps: upgrade openssl sources to 1.1.1g (Hassaan Pasha)
  [#32971](#32971)
- doc: add juanarbol as collaborator (Juan José Arboleda)
  [#32906](#32906)
- http: doc deprecate abort and improve docs (Robert Nagy)
  [#32807](#32807)
- module: do not warn when accessing `__esModule` of unfinished exports
  (Anna Henningsen) [#33048](#33048)
- n-api: detect deadlocks in thread-safe function (Gabriel Schulhof)
  [#32860](#32860)
- src: deprecate embedder APIs with replacements (Anna Henningsen)
  [#32858](#32858)
- stream:
  - don't emit end after close (Robert Nagy)
    [#33076](#33076)
  - don't wait for close on legacy streams (Robert Nagy)
    [#33058](#33058)
  - pipeline should only destroy un-finished streams (Robert Nagy)
    [#32968](#32968)
- vm: add importModuleDynamically option to compileFunction (Gus Caplan)
  [#32985](#32985)

PR-URL: #33103
BethGriggs added a commit that referenced this pull request Apr 29, 2020
@BethGriggs
2020-04-29, Version 14.1.0 (Current)
49db211 
Notable changes:

- deps: upgrade openssl sources to 1.1.1g (Hassaan Pasha)
  [#32971](#32971)
- doc: add juanarbol as collaborator (Juan José Arboleda)
  [#32906](#32906)
- http: doc deprecate abort and improve docs (Robert Nagy)
  [#32807](#32807)
- module: do not warn when accessing `__esModule` of unfinished exports
  (Anna Henningsen) [#33048](#33048)
- n-api: detect deadlocks in thread-safe function (Gabriel Schulhof)
  [#32860](#32860)
- src: deprecate embedder APIs with replacements (Anna Henningsen)
  [#32858](#32858)
- stream:
  - don't emit end after close (Robert Nagy)
    [#33076](#33076)
  - don't wait for close on legacy streams (Robert Nagy)
    [#33058](#33058)
  - pipeline should only destroy un-finished streams (Robert Nagy)
    [#32968](#32968)
- vm: add importModuleDynamically option to compileFunction (Gus Caplan)
  [#32985](#32985)

PR-URL: #33103
dr-js added a commit to dr-js/dr-js that referenced this pull request Jul 10, 2020
@dr-js
@dr-js/core@0.3.0
4b4620e 
notable change:
- deprecated: `objectDeleteUndefined` from `common/immutable/Object`
- deprecated: node: `requestAsync` from `node/net`
- deprecated: node: `urlString` option in `createWebSocketClient` from `node/server/WebSocket/WebSocketClient`
- break: sort `Async*` code:
  - use `AsyncFuncQueue` instead of `AsyncTaskQueue` (previous one)
  - use `AsyncLane` with `Async*Queue` support instead of `AsyncTaskLane` (previous one)
  - use `AsyncTaskQueue` instead of `AsyncTaskRunner`
  - use `AsyncTaskQueue + AsyncLane` instead of `AsyncTaskRunnerCluster`
- break: use `prettyStringifyTreeNode` instead of `prettyStringifyTree`
- break: node: mass code sort for `node/file/`:
  - drop `node/file/function`, use `fs` directly
  - default to `getPathLstat` so most symlink will be kept
  - sort `node/file/Directory`:
    - use `getDirInfoList` instead of `getDirectorySubInfoList`
    - use `getDirInfoTree` instead of `getDirectoryInfoTree`
    - use `walkDirInfoTreeAsync` instead of `walkDirectoryInfoTree`
    - use `walkDirInfoTreeBottomUpAsync` instead of `walkDirectoryInfoTreeBottomUp`
    - use `copyDirInfoTree` instead of `copyDirectoryInfoTree`
    - use `renameDirInfoTree` instead of `renameDirectoryInfoTree`
    - use `deleteDirInfoTree` instead of `deleteDirectoryInfoTree`
- break: node: mass code sort in `node/data/Stream`:
  - use `readableStreamToBufferAsync` instead of `receiveBufferAsync` from `node/data/Buffer`
  - use `writeBufferToStreamAsync` instead of `sendBufferAsync` from `node/data/Buffer`
  - use `waitStreamStopAsync(setupStreamPipe( ... ))` instead of `pipeStreamAsync`
  - use `readlineOfStreamAsync` instead of `createReadlineFromStreamAsync`
- break: node: use `getDefaultOpenCommandList` instead of `getDefaultOpen`
- break: node: use `request.destroy()|request.destroyed` instead of `request.abort()|request.aborted` follow: nodejs/node#32807
- break: node: use `response.writableEnded` (need `nodejs@>=12.9`)
- break: browser: use `onProgressUpload/onProgressDownload` instead of `onUploadProgress/onDownloadProgress` for `fetchLikeRequest`
- fix: ci: git CRLF config & longer test timeout
- fix: ci: patch `libgbm1` for `puppeteer@3`, check: puppeteer/puppeteer#5674
- fix: better handle WebSocket `doUpgradeSocket` error
- fix: sanity test for `nodejs@14`
- fix: flaky test `fetchLikeRequest() option: timeout` in `net.test.js`
- fix: code lint for `eslint-config-almost-standard-v14`
- add: `onProgressUpload/onProgressDownload` for both `fetchLikeRequest`
- add: `AsyncTask` and `AsyncTaskQueue`
- add: `catchPromise` to `common/error`
- add: `.sanity.test` for some peace of mind
- add: `typeNameOf` to `common/format`
- add: tree-like structure support in `common/data/Tree`
- add: node: support pass `ReadableStream` as `body` to `fetchLikeRequest`
- add: node: `isReadableStream/isWritableStream`
- add: node: `ResolveCommand` moved from `@dr-js/node`
- add: node: TLS server session ticket rotation
- add: browser: `browser/net.test.js`
- add: bin: percent for mode `fetch`
- ci: update `.gitignore` & `.github/workflows/ci-test.yml`
- better `error.stack` for `withTimeoutPromise|withTimeoutAsync`
- simplify `createInsideOutPromise` code
- sort test file naming
- package update

notable change from `0.2.1-dev.*`:
- add: `isRegExp|regexp` in `common/check|verify`
- add: `createMarkReplacer` to `common/string`
- add: node: `closeAsync|readAsync|writeAsync|readlinkAsync|symlinkAsync` to `node/file/function`
- add: node: `resolvePath` to `createOptionGetter` in `node/module/Option/preset`
- fix: node: leaked `error` event in `createReadlineFromStreamAsync`
- node: update `buffer.readUInt*()` usage for removed `noAssert` since `nodejs@10`
- node: sort `node/module/Option/preset` code
- sort build script
- package update
dr-js added a commit to dr-js/dr-node that referenced this pull request Jul 11, 2020
@dr-js
@dr-js/node@0.3.0
c1bb071 
notable change:
- deprecated: use `PATH_RENAME` instead of `PATH_MOVE`
- deprecated: `fileTLS*` option of `configureServerPack` from `module/ServerPack`, use `TLSSNIConfig|TLSDHParam` instead
- deprecated: bin: `file-TLS-*` option, use `TLS-SNI-config|TLS-dhparam` instead
- break: use `ResolveCommand` from `@dr-js/core`
- break: use `request.destroy()|request.destroyed` instead of `request.abort()|request.aborted` follow: nodejs/node#32807
- break: drop `TaskAction` module and `TaskRunner` feature to allow later to add improved version
- break: bin: merge `compress/extract` to use `module/Software/archive`
- break: bin: use `TLS-SNI-config` instead of `https` to enable https
- add: `REGEXP_AUTO|REGEXP_T7Z|REGEXP_TXZ` to `module/Software/archive`
- add: `REGEXP_NPM_TAR|extractPackageJson` to `module/Software/npmTar`
- add: `toPackageTgzName` to `module/Software/npm`
- add: `AuthCommonOption` to `server/feature/Auth/option` to allow set `authKey`
- add: `RuntimeDump` for getting runtime heap status & use `setupPackageSIGUSR2`
- add: `module/Software/archive` and `module/Software/npmTar`
- add: `module/TerminalStatusBar`
- add: `module/TerminalColor` & `module/Software/npm` from `@dr-js/dev`
- add: `createCommandWrap` for `module/Software/7z|git|tar` to allow reset command
- add: feature: `WebSocketTunnelDev` (under DEV)
- fix: code lint for `eslint-config-almost-standard-v14`
- ci: run on all 3 platform & test `nodejs@14`
- script sort
- package update

notable change from `0.2.1-dev.*`:
- add: `module/FsPack`
- add: `module/ResolveCommand`
- add: `module/RunDetached`
- add: `resolveCommand|resolveCommandAsync` and `pingRaceUrlList|pingStatUrlList`
- add: explorer: add route `/timestamp` for server time & ping
- add: bin: mode `ping-race|ping-stat` & option `debug-route`
- fix: `TLSSNIConfig` value normalize in `ServerPack`
- sort script
- package update
@aduh95 aduh95 mentioned this pull request Oct 6, 2020
Closed
3 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@addaleax addaleax addaleax approved these changes

@himself65 himself65 himself65 approved these changes

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

@trivikr trivikr trivikr approved these changes

@bnoordhuis bnoordhuis Awaiting requested review from bnoordhuis

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. notable-change PRs with changes that should be highlighted in changelogs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@ronag @himself65 @nodejs-github-bot @mcollina @addaleax @jasnell @Trott @trivikr @targos @BridgeAR