crypto: improve crypto error messages#3100

thefourtheye · 2015-09-28T06:29:37Z

Introduce a new MACRO to check if the data is a String object and
updated existing MACROs to include the actual object description to
be printed if there is an error.

cc @nodejs/crypto

YurySolovyov · 2015-09-29T15:59:58Z

src/node_crypto.cc

maybe "Parameter must be an integer value" ? since it looks like it is the only one possible param here

@YuriSolovyov Ya, that is why I left it as the function name hints. I don't have a strong opinion about this name anyway.

thefourtheye · 2015-10-06T13:32:02Z

Bump!

bnoordhuis · 2015-10-06T14:58:38Z

src/node_crypto.cc

obj is a pretty bad name. Maybe call it prefix?

thefourtheye · 2015-10-10T04:36:41Z

@bnoordhuis I fixed as per your suggestions. PTAL.

bnoordhuis · 2015-10-10T11:15:31Z

src/node_crypto.cc

"ECDH curve name" here and below?

bnoordhuis · 2015-10-10T11:18:58Z

Still quite a few places where words are Incongruously Capitalized.

thefourtheye · 2015-10-10T12:56:46Z

@bnoordhuis Sorry, I think I misunderstood your initial comments. I just thought that names are inconsistent. I didn't think about the case. I hope I updated all the places properly this time. PTAL.

thefourtheye · 2015-10-25T18:46:24Z

Bump!

thefourtheye · 2016-03-25T10:40:55Z

Okay, now I addressed @bnoordhuis's comments.

jasnell · 2016-03-26T04:21:47Z

LGTM if @bnoordhuis is happy and CI is green.
New CI: https://ci.nodejs.org/job/node-test-pull-request/2072/

jasnell · 2016-03-26T04:22:11Z

semver-major because of the changes to the error messages.

bnoordhuis · 2016-03-26T16:55:09Z

LGTM

indutny · 2016-03-26T16:58:00Z

LGTM

Introduce a new MACRO to check if the data is a String object and update existing MACROs to include the actual object description to be printed in case of an error. PR-URL: #3100 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]>

thefourtheye · 2016-03-26T18:17:50Z

Thanks for the reviews :-) Landed in 41feaa8.

The following significant (semver-major) changes have been made since the previous Node v5.0.0 release. * Buffer * New Buffer constructors have been added [#4682](#4682) * Previously deprecated Buffer APIs are removed [#5048](#5048), [#4594](#4594) * Improved error handling [#4514](#4514) * Cluster * Worker emitted as first argument in 'message' event [#5361](#5361). * Crypto * Improved error handling [#3100](#3100), [#5611](#5611) * Simplified Certificate class bindings [#5382](#5382) * Improved control over FIPS mode [#5181](#5181) * pbkdf2 digest overloading is deprecated [#4047](#4047) * Dependencies * Reintroduce shared c-ares build support [#5775](#5775). * V8 updated to 5.0.71.31 [#6111](#6111). * DNS * Add resolvePtr API to query plain DNS PTR records [#4921](#4921). * Domains * Clear stack when no error handler [#4659](#4659). * File System * The `fs.realpath()` and `fs.realpathSync()` methods have been updated to use a more efficient libuv implementation. This change includes the removal of the `cache` argument and the method can throw new errors [#3594](#3594) * FS apis can now accept and return paths as Buffers [#5616](#5616). * Error handling and type checking improvements [#5616](#5616), [#5590](#5590), [#4518](#4518), [#3917](#3917). * fs.read's string interface is deprecated [#4525](#4525) * HTTP * 'clientError' can now be used to return custom errors from an HTTP server [#4557](#4557). * Modules * Current directory is now prioritized for local lookups [#5689](#5689) * Symbolic links are preserved when requiring modules [#5950](#5950) * Net * DNS hints no longer implicitly set [#6021](#6021). * Improved error handling and type checking [#5981](#5981), [#5733](#5733), [#2904](#2904) * Path * Improved type checking [#5348](#5348). * Process * Introduce process warnings API [#4782](#4782). * Throw exception when non-function passed to nextTick [#3860](#3860). * Readline * Emit key info unconditionally [#6024](#6024) * REPL * Assignment to `_` will emit a warning. [#5535](#5535) * Timers * Fail early when callback is not a function [#4362](#4362) * TLS * Rename 'clientError' to 'tlsClientError' [#4557](#4557) * SHA1 used for sessionIdContext [#3866](#3866) * TTY * Previously deprecated setRawMode wrapper is removed [#2528](#2528). * Util * Changes to Error object formatting [#4582](#4582). * Windows * Windows XP and Vista are no longer supported [#5167](#5167), [#5167](#5167).

The following significant (semver-major) changes have been made since the previous Node v5.0.0 release. * Buffer * New Buffer constructors have been added [#4682](#4682) * Previously deprecated Buffer APIs are removed [#5048](#5048), [#4594](#4594) * Improved error handling [#4514](#4514) * Cluster * Worker emitted as first argument in 'message' event [#5361](#5361). * Crypto * Improved error handling [#3100](#3100), [#5611](#5611) * Simplified Certificate class bindings [#5382](#5382) * Improved control over FIPS mode [#5181](#5181) * pbkdf2 digest overloading is deprecated [#4047](#4047) * Dependencies * Reintroduce shared c-ares build support [#5775](#5775). * V8 updated to 5.0.71.31 [#6111](#6111). * DNS * Add resolvePtr API to query plain DNS PTR records [#4921](#4921). * Domains * Clear stack when no error handler [#4659](#4659). * File System * The `fs.realpath()` and `fs.realpathSync()` methods have been updated to use a more efficient libuv implementation. This change includes the removal of the `cache` argument and the method can throw new errors [#3594](#3594) * FS apis can now accept and return paths as Buffers [#5616](#5616). * Error handling and type checking improvements [#5616](#5616), [#5590](#5590), [#4518](#4518), [#3917](#3917). * fs.read's string interface is deprecated [#4525](#4525) * HTTP * 'clientError' can now be used to return custom errors from an HTTP server [#4557](#4557). * Modules * Current directory is now prioritized for local lookups [#5689](#5689) * Symbolic links are preserved when requiring modules [#5950](#5950) * Net * DNS hints no longer implicitly set [#6021](#6021). * Improved error handling and type checking [#5981](#5981), [#5733](#5733), [#2904](#2904) * OS X * MACOSX_DEPLOYMENT_TARGET has been bumped up to 10.7 [#6402](#6402). * Path * Improved type checking [#5348](#5348). * Process * Introduce process warnings API [#4782](#4782). * Throw exception when non-function passed to nextTick [#3860](#3860). * Readline * Emit key info unconditionally [#6024](#6024) * REPL * Assignment to `_` will emit a warning. [#5535](#5535) * Timers * Fail early when callback is not a function [#4362](#4362) * TLS * Rename 'clientError' to 'tlsClientError' [#4557](#4557) * SHA1 used for sessionIdContext [#3866](#3866) * TTY * Previously deprecated setRawMode wrapper is removed [#2528](#2528). * Util * Changes to Error object formatting [#4582](#4582). * Windows * Windows XP and Vista are no longer supported [#5167](#5167), [#5167](#5167).

thefourtheye added crypto Issues and PRs related to the crypto subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. test Issues and PRs related to the tests. labels Sep 28, 2015

thefourtheye force-pushed the improve-crypto-error-messages branch from c047adf to 7725b33Compare September 28, 2015 10:52

YurySolovyov reviewed Sep 29, 2015
View reviewed changes

bnoordhuis reviewed Oct 6, 2015
View reviewed changes

src/node_crypto.cc Outdated
Copy link
Member
bnoordhuisOct 6, 2015
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
obj is a pretty bad name. Maybe call it prefix?

thefourtheye force-pushed the improve-crypto-error-messages branch 2 times, most recently from e3f42c2 to bba8b98Compare October 10, 2015 04:16

bnoordhuis reviewed Oct 10, 2015
View reviewed changes

src/node_crypto.cc Outdated
Copy link
Member
bnoordhuisOct 10, 2015
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ECDH curve name" here and below?

thefourtheye force-pushed the improve-crypto-error-messages branch from fb918ea to be0e7deCompare October 10, 2015 12:40

thefourtheye force-pushed the improve-crypto-error-messages branch from be0e7de to e0ae164Compare October 10, 2015 12:45

thefourtheye force-pushed the improve-crypto-error-messages branch from e0ae164 to a99236cCompare October 10, 2015 12:55

thefourtheye force-pushed the improve-crypto-error-messages branch from ad720dd to d9dabd2Compare March 25, 2016 10:39

jasnell added semver-major PRs that contain breaking changes and should be released in the next major version. and removed stalled Issues and PRs that are stalled. labels Mar 26, 2016

thefourtheye closed this Mar 26, 2016

thefourtheye deleted the improve-crypto-error-messages branch March 26, 2016 18:18

jasnell mentioned this pull request Apr 19, 2016
What is new in v6? #6264
Closed

jasnell mentioned this pull request Apr 26, 2016
2016-04-26, Version 6.0.0 (Current) Release #6383
Closed

lpinca mentioned this pull request Oct 14, 2017
Explain error messages better (be developer friendly) websockets/ws#429
Closed

aliscco mentioned this pull request Jun 22, 2023
[Snyk] Security upgrade karma from 1.3.0 to 3.0.0 aliscco/alisco-node#496
Open

Uh oh!

crypto: improve crypto error messages#3100

crypto: improve crypto error messages #3100

Uh oh!

Conversation

thefourtheye commented Sep 28, 2015

Uh oh!

YurySolovyovSep 29, 2015

Choose a reason for hiding this comment

Uh oh!

thefourtheyeOct 6, 2015

Choose a reason for hiding this comment

Uh oh!

thefourtheye commented Oct 6, 2015

Uh oh!

bnoordhuisOct 6, 2015

Choose a reason for hiding this comment

Uh oh!

thefourtheye commented Oct 10, 2015

Uh oh!

bnoordhuisOct 10, 2015

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented Oct 10, 2015

Uh oh!

thefourtheye commented Oct 10, 2015

Uh oh!

thefourtheye commented Oct 25, 2015

Uh oh!

thefourtheye commented Mar 25, 2016

Uh oh!

jasnell commented Mar 26, 2016

Uh oh!

jasnell commented Mar 26, 2016

Uh oh!

bnoordhuis commented Mar 26, 2016

Uh oh!

indutny commented Mar 26, 2016

Uh oh!

thefourtheye commented Mar 26, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants