Skip to content

http,stream: make virtual methods throw an error - #31912

Closed
lpinca wants to merge 1 commit into
nodejs:masterfrom
lpinca:throw/err-method-not-implemented
Closed

lpinca wants to merge 1 commit into
nodejs:masterfrom
lpinca:throw/err-method-not-implemented

Conversation

@lpinca

@lpinca lpinca commented Feb 22, 2020

Copy link
Copy Markdown
Member

Make virtual methods throw an ERR_METHOD_NOT_IMPLEMENTED error instead
of emitting it.

The error is not recoverable and the only way to handle it is to
override the method.

Refs: #31818 (review)

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

Make virtual methods throw an ERR_METHOD_NOT_IMPLEMENTED error instead
of emitting it.

The error is not recoverable and the only way to handle it is to
override the method.

Refs: nodejs#31818 (review)
@nodejs-github-bot nodejs-github-bot added http Issues and PRs related to the http subsystem. stream Issues and PRs related to Node.js streams. labels Feb 22, 2020
@lpinca lpinca added the semver-major PRs that contain breaking changes and should be released in the next major version. label Feb 22, 2020
@lpinca

lpinca commented Feb 22, 2020 •

Copy link
Copy Markdown
Member Author

cc: @nodejs/streams @ronag

@lpinca
lpinca requested a review from ronag February 22, 2020 13:29
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@benjamingr

Copy link
Copy Markdown
Member

LGTM with semver-major and citgm

@nodejs-github-bot

nodejs-github-bot commented Feb 23, 2020 •

Copy link
Copy Markdown
Collaborator

@lpinca

lpinca commented Mar 6, 2020

Copy link
Copy Markdown
Member Author

lpinca added a commit that referenced this pull request Mar 7, 2020
Make virtual methods throw an ERR_METHOD_NOT_IMPLEMENTED error instead
of emitting it.

The error is not recoverable and the only way to handle it is to
override the method.

PR-URL: #31912
Refs: #31818 (review)
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@lpinca

lpinca commented Mar 7, 2020

Copy link
Copy Markdown
Member Author

Landed in 6f0ec79.

@lpinca lpinca closed this Mar 7, 2020
@lpinca
lpinca deleted the throw/err-method-not-implemented branch March 7, 2020 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http Issues and PRs related to the http subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. stream Issues and PRs related to Node.js streams.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants