src: add .code and SSL specific error properties#25093
Closed
sam-github wants to merge 1 commit intonodejs:masterfrom
Closed
src: add .code and SSL specific error properties#25093sam-github wants to merge 1 commit intonodejs:masterfrom
sam-github wants to merge 1 commit intonodejs:masterfrom
Conversation
jasnell
reviewed
Dec 18, 2018
jasnell
reviewed
Dec 18, 2018
Contributor
Author
|
@jasnell PTAL |
jasnell
approved these changes
Dec 18, 2018
35dc539 to
18c6c1f
Compare
danbev
approved these changes
Dec 21, 2018
Contributor
|
@sam-github LGTM apart from I'm seeing the same test failures locally as reported by the CI run. Details$ out/Release/node --tls-v1.1 /Users/danielbevenius/work/nodejs/node/test/parallel/test-tls-cli-min-version-1.1.js
assert.js:86
throw new AssertionError(obj);
^
AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected
+ undefined
- 'ERR_SSL_VERSION_TOO_LOW'
at common.mustCall (/Users/danielbevenius/work/nodejs/node/test/parallel/test-tls-min-max-version.js:31:14)
at /Users/danielbevenius/work/nodejs/node/test/common/index.js:373:15
at /Users/danielbevenius/work/nodejs/node/test/common/index.js:373:15
at maybeCallback (/Users/danielbevenius/work/nodejs/node/test/fixtures/tls-connect.js:97:7)
at Server.<anonymous> (/Users/danielbevenius/work/nodejs/node/test/fixtures/tls-connect.js:84:7)
at Server.emit (events.js:188:13)
at TLSSocket.onSocketTLSError (_tls_wrap.js:728:29)
at TLSSocket.emit (events.js:188:13)
at TLSSocket._tlsError (_tls_wrap.js:608:8)
at TLSSocket.emit (events.js:188:13)
|
SSL errors have a long structured message, but lacked the standard .code property which can be used for stable comparisons. Add a `code` property, as well as the 3 string components of an SSL error: `reason`, `library`, and `function`.
18c6c1f to
04e841c
Compare
Contributor
Author
|
@danbev Oops, reordered the error number peek to after the error message long-form printf while addressing some comments, which doesn't work (printf clears the err number). Tests are passing again locally, lets see how CI does. |
Contributor
Author
Contributor
Author
|
Landed in ca9c0c9 |
sam-github
added a commit
that referenced
this pull request
Dec 27, 2018
SSL errors have a long structured message, but lacked the standard .code property which can be used for stable comparisons. Add a `code` property, as well as the 3 string components of an SSL error: `reason`, `library`, and `function`. PR-URL: #25093 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Member
|
Should this be backported to |
4 tasks
sam-github
added a commit
to sam-github/node
that referenced
this pull request
Jan 7, 2019
SSL errors have a long structured message, but lacked the standard .code property which can be used for stable comparisons. Add a `code` property, as well as the 3 string components of an SSL error: `reason`, `library`, and `function`. Backport-PR-URL: https://github.com/nodejs/node/25376 PR-URL: nodejs#25093 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Contributor
Author
|
Depends on #24729, semver-major, so not a backport candidate. |
refack
pushed a commit
to refack/node
that referenced
this pull request
Jan 14, 2019
SSL errors have a long structured message, but lacked the standard .code property which can be used for stable comparisons. Add a `code` property, as well as the 3 string components of an SSL error: `reason`, `library`, and `function`. PR-URL: nodejs#25093 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
4 tasks
sam-github
added a commit
to sam-github/node
that referenced
this pull request
Mar 28, 2019
SSL errors have a long structured message, but lacked the standard .code property which can be used for stable comparisons. Add a `code` property, as well as the 3 string components of an SSL error: `reason`, `library`, and `function`. PR-URL: nodejs#25093 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
4 tasks
sam-github
added a commit
to sam-github/node
that referenced
this pull request
Apr 1, 2019
SSL errors have a long structured message, but lacked the standard .code property which can be used for stable comparisons. Add a `code` property, as well as the 3 string components of an SSL error: `reason`, `library`, and `function`. PR-URL: nodejs#25093 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
sam-github
added a commit
to sam-github/node
that referenced
this pull request
Apr 11, 2019
SSL errors have a long structured message, but lacked the standard .code property which can be used for stable comparisons. Add a `code` property, as well as the 3 string components of an SSL error: `reason`, `library`, and `function`. PR-URL: nodejs#25093 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BethGriggs
pushed a commit
that referenced
this pull request
Apr 15, 2019
SSL errors have a long structured message, but lacked the standard .code property which can be used for stable comparisons. Add a `code` property, as well as the 3 string components of an SSL error: `reason`, `library`, and `function`. Backport-PR-URL: #26951 PR-URL: #25093 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Merged
codebytere
added a commit
that referenced
this pull request
Apr 19, 2019
Notable changes: * deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) [#19794](#19794) * src: add .code and SSL specific error properties (Sam Roberts) [#25093](#25093) * tls: * add --tls-min-v1.2 CLI switch (Sam Roberts) [#26951](#26951) * supported shared openssl 1.1.0 (Sam Roberts) [#26951](#26951) * revert default max toTLSv1.2 (Sam Roberts) [#26951](#26951) * revert change to invalid protocol error type (Sam Roberts) [#26951](#26951) * support TLSv1.3 (Sam Roberts) [#26209](#26209) * add code for ERR\_TLS\_INVALID\_PROTOCOL\_METHOD (Sam Roberts) [#24729](#24729)
codebytere
added a commit
that referenced
this pull request
Apr 19, 2019
Notable changes: * deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) [#19794](#19794) * src: add .code and SSL specific error properties (Sam Roberts) [#25093](#25093) * tls: * add --tls-min-v1.2 CLI switch (Sam Roberts) [#26951](#26951) * supported shared openssl 1.1.0 (Sam Roberts) [#26951](#26951) * revert default max toTLSv1.2 (Sam Roberts) [#26951](#26951) * revert change to invalid protocol error type (Sam Roberts) [#26951](#26951) * support TLSv1.3 (Sam Roberts) [#26209](#26209) * add code for ERR\_TLS\_INVALID\_PROTOCOL\_METHOD (Sam Roberts) [#24729](#24729)
This was referenced Apr 23, 2019
codebytere
added a commit
that referenced
this pull request
Apr 30, 2019
Notable changes: * deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) [#19794](#19794) * src: add .code and SSL specific error properties (Sam Roberts) [#25093](#25093) * tls: * add --tls-min-v1.2 CLI switch (Sam Roberts) [#26951](#26951) * supported shared openssl 1.1.0 (Sam Roberts) [#26951](#26951) * revert default max toTLSv1.2 (Sam Roberts) [#26951](#26951) * revert change to invalid protocol error type (Sam Roberts) [#26951](#26951) * support TLSv1.3 (Sam Roberts) [#26209](#26209) * add code for ERR\_TLS\_INVALID\_PROTOCOL\_METHOD (Sam Roberts) [#24729](#24729) PR-URL: #27314
codebytere
added a commit
that referenced
this pull request
Apr 30, 2019
Notable changes: * deps: add s390 asm rules for OpenSSL-1.1.1 (Shigeki Ohtsu) [#19794](#19794) * src: add .code and SSL specific error properties (Sam Roberts) [#25093](#25093) * tls: * add --tls-min-v1.2 CLI switch (Sam Roberts) [#26951](#26951) * supported shared openssl 1.1.0 (Sam Roberts) [#26951](#26951) * revert default max toTLSv1.2 (Sam Roberts) [#26951](#26951) * revert change to invalid protocol error type (Sam Roberts) [#26951](#26951) * support TLSv1.3 (Sam Roberts) [#26209](#26209) * add code for ERR\_TLS\_INVALID\_PROTOCOL\_METHOD (Sam Roberts) [#24729](#24729) PR-URL: #27314
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SSL errors have a long structured message, but lacked the standard .code
property which can be used for stable comparisons. Add a
codeproperty, as well as the 3 string components of an SSL error:
reason,library, andfunction.This is WIP, TBD is move the property strings to the isolate, and add docs and more tests.
I've seen a reasonable amount of code that is doing string matching on the error string, looking for the reason part, I think exposing the properties makes more sense. Also, I think the intention is for all Errors to expose
.codeproperties for stable programmatic inspection.I've gone some way to exposing these properties for node_crypto.cc, too, but I'm not done. Once this PR lands I'll keep working on it.
/to @nodejs/crypto
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes