Skip to content

crypto: assign and use ERR_CRYPTO_UNKNOWN_CIPHER#31437

Closed
tniessen wants to merge 2 commits into
nodejs:masterfrom
tniessen:crypto-add-errorcryptounknowncipher
Closed

crypto: assign and use ERR_CRYPTO_UNKNOWN_CIPHER#31437
tniessen wants to merge 2 commits into
nodejs:masterfrom
tniessen:crypto-add-errorcryptounknowncipher

Conversation

@tniessen
Copy link
Copy Markdown
Member

@tniessen tniessen commented Jan 21, 2020

This replaces Errors with the message 'Unknown cipher' with new errors with the same message, but assigns a new code ERR_CRYPTO_UNKNOWN_CIPHER. This should not break existing applications since it only adds the .code property, and should therefore not be semver-major.

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
@tniessen tniessen added crypto Issues and PRs related to the crypto subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Jan 21, 2020
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jan 21, 2020
cjihrig
cjihrig approved these changes Jan 21, 2020
View reviewed changes
Comment thread doc/api/errors.md Outdated
@tniessen @cjihrig
fixup! crypto: assign and use ERR_CRYPTO_UNKNOWN_CIPHER
8bba027 
Co-Authored-By: Colin Ihrig <cjihrig@gmail.com>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Jan 21, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/28545/
@tniessen tniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 21, 2020
@Trott
Copy link
Copy Markdown
Member

Trott commented Jan 23, 2020

Landed in 8c313ce
@Trott Trott closed this Jan 23, 2020
Trott pushed a commit that referenced this pull request Jan 23, 2020
@tniessen @Trott
crypto: assign and use ERR_CRYPTO_UNKNOWN_CIPHER
8c313ce 
PR-URL: #31437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Feb 17, 2020
@tniessen @codebytere
crypto: assign and use ERR_CRYPTO_UNKNOWN_CIPHER
4591202 
PR-URL: #31437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@codebytere codebytere mentioned this pull request Feb 17, 2020
Merged
@BethGriggs
Copy link
Copy Markdown
Member

BethGriggs commented Apr 14, 2020

@codebytere is this still blocked for v12.x?
@tniessen
Copy link
Copy Markdown
Member Author

tniessen commented Apr 15, 2020

I don't know. I am still not sure if this is considered a breaking change or not (I didn't assume so, but IIRC @jasnell did). Ref: #31873 (comment)
@jasnell
Copy link
Copy Markdown
Member

jasnell commented Apr 15, 2020

Per policy assigning new Error codes is semver-major typically because they also come with a change to the error message. After that initial assignment, changing the message is patch or minor. Not dead set on that policy now that error codes have become a natural normal thing.
@targos targos removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-blocked-v12.x labels Apr 25, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
@tniessen @targos
crypto: assign and use ERR_CRYPTO_UNKNOWN_CIPHER
9718497 
PR-URL: nodejs#31437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Apr 28, 2020
@tniessen @targos
crypto: assign and use ERR_CRYPTO_UNKNOWN_CIPHER
30633ac 
PR-URL: #31437
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request May 2, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core.