crypto: improve crypto error messages by thefourtheye · Pull Request #3100 · nodejs/node (original) (raw)
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
Issues and PRs related to the crypto subsystem.
Issues and PRs that require attention from people who are familiar with C++.
Issues and PRs related to the tests.
labels
| @@ -818,7 +835,7 @@ void SecureContext::SetOptions(const FunctionCallbackInfo& args) { |
|---|
| SecureContext* sc = Unwrap(args.Holder()); |
| if (args.Length() != 1 | |
| return sc->env()->ThrowTypeError("Bad parameter"); |
| return sc->env()->ThrowTypeError("Options must be an integer value"); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe "Parameter must be an integer value" ? since it looks like it is the only one possible param here
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@YuriSolovyov Ya, that is why I left it as the function name hints. I don't have a strong opinion about this name anyway.
| @@ -33,17 +33,24 @@ |
|---|
| #define OPENSSL_CONST |
| #endif |
| #define THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(val) \ |
| #define THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(val, obj) \ |
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?
| if (args.Length() != 1 || !args[0]->IsString()) | | -------------------------------------------------------------------- | | return env->ThrowTypeError("First argument should be a string"); | | if (args.Length() != 1) | | return env->ThrowTypeError("ECDH Curve name argument is mandatory"); |
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?
Still quite a few places where words are Incongruously Capitalized.
@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.
PRs that contain breaking changes and should be released in the next major version.
and removed stalled
Issues and PRs that are stalled.
labels
semver-major because of the changes to the error messages.
1 similar comment
thefourtheye added a commit that referenced this pull request
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 info@bnoordhuis.nl Reviewed-By: Fedor Indutny fedor.indutny@gmail.com Reviewed-By: James M Snell jasnell@gmail.com
Thanks for the reviews :-) Landed in 41feaa8.
jasnell added a commit that referenced this pull request
The following significant (semver-major) changes have been made since the previous Node v5.0.0 release.
- Buffer
- Cluster
- Worker emitted as first argument in 'message' event #5361.
- Crypto
- Dependencies
- DNS
- Add resolvePtr API to query plain DNS PTR records #4921.
- Domains
- Clear stack when no error handler #4659.
- File System
- The
fs.realpath()andfs.realpathSync()methods have been updated to use a more efficient libuv implementation. This change includes the removal of thecacheargument and the method can throw new errors #3594 - FS apis can now accept and return paths as Buffers #5616.
- Error handling and type checking improvements #5616, #5590, #4518, #3917.
- fs.read's string interface is deprecated #4525
- The
- HTTP
- 'clientError' can now be used to return custom errors from an HTTP server #4557.
- Modules
- Net
- Path
- Improved type checking #5348.
- Process
- Readline
- Emit key info unconditionally #6024
- REPL
- Assignment to
_will emit a warning. #5535
- Assignment to
- Timers
- Fail early when callback is not a function #4362
- TLS
- TTY
- Previously deprecated setRawMode wrapper is removed #2528.
- Util
- Changes to Error object formatting #4582.
- Windows
jasnell added a commit that referenced this pull request
The following significant (semver-major) changes have been made since the previous Node v5.0.0 release.
- Buffer
- Cluster
- Worker emitted as first argument in 'message' event #5361.
- Crypto
- Dependencies
- DNS
- Add resolvePtr API to query plain DNS PTR records #4921.
- Domains
- Clear stack when no error handler #4659.
- File System
- The
fs.realpath()andfs.realpathSync()methods have been updated to use a more efficient libuv implementation. This change includes the removal of thecacheargument and the method can throw new errors #3594 - FS apis can now accept and return paths as Buffers #5616.
- Error handling and type checking improvements #5616, #5590, #4518, #3917.
- fs.read's string interface is deprecated #4525
- The
- HTTP
- 'clientError' can now be used to return custom errors from an HTTP server #4557.
- Modules
- Net
- Path
- Improved type checking #5348.
- Process
- Readline
- Emit key info unconditionally #6024
- REPL
- Assignment to
_will emit a warning. #5535
- Assignment to
- Timers
- Fail early when callback is not a function #4362
- TLS
- TTY
- Previously deprecated setRawMode wrapper is removed #2528.
- Util
- Changes to Error object formatting #4582.
- Windows
jasnell added a commit that referenced this pull request
The following significant (semver-major) changes have been made since the previous Node v5.0.0 release.
- Buffer
- Cluster
- Worker emitted as first argument in 'message' event #5361.
- Crypto
- Dependencies
- DNS
- Add resolvePtr API to query plain DNS PTR records #4921.
- Domains
- Clear stack when no error handler #4659.
- File System
- The
fs.realpath()andfs.realpathSync()methods have been updated to use a more efficient libuv implementation. This change includes the removal of thecacheargument and the method can throw new errors #3594 - FS apis can now accept and return paths as Buffers #5616.
- Error handling and type checking improvements #5616, #5590, #4518, #3917.
- fs.read's string interface is deprecated #4525
- The
- HTTP
- 'clientError' can now be used to return custom errors from an HTTP server #4557.
- Modules
- Net
- OS X
- MACOSX_DEPLOYMENT_TARGET has been bumped up to 10.7 #6402.
- Path
- Improved type checking #5348.
- Process
- Readline
- Emit key info unconditionally #6024
- REPL
- Assignment to
_will emit a warning. #5535
- Assignment to
- Timers
- Fail early when callback is not a function #4362
- TLS
- TTY
- Previously deprecated setRawMode wrapper is removed #2528.
- Util
- Changes to Error object formatting #4582.
- Windows
jasnell added a commit that referenced this pull request
The following significant (semver-major) changes have been made since the previous Node v5.0.0 release.
- Buffer
- Cluster
- Worker emitted as first argument in 'message' event #5361.
- Crypto
- Dependencies
- DNS
- Add resolvePtr API to query plain DNS PTR records #4921.
- Domains
- Clear stack when no error handler #4659.
- File System
- The
fs.realpath()andfs.realpathSync()methods have been updated to use a more efficient libuv implementation. This change includes the removal of thecacheargument and the method can throw new errors #3594 - FS apis can now accept and return paths as Buffers #5616.
- Error handling and type checking improvements #5616, #5590, #4518, #3917.
- fs.read's string interface is deprecated #4525
- The
- HTTP
- 'clientError' can now be used to return custom errors from an HTTP server #4557.
- Modules
- Net
- OS X
- MACOSX_DEPLOYMENT_TARGET has been bumped up to 10.7 #6402.
- Path
- Improved type checking #5348.
- Process
- Readline
- Emit key info unconditionally #6024
- REPL
- Assignment to
_will emit a warning. #5535
- Assignment to
- Timers
- Fail early when callback is not a function #4362
- TLS
- TTY
- Previously deprecated setRawMode wrapper is removed #2528.
- Util
- Changes to Error object formatting #4582.
- Windows
jasnell added a commit that referenced this pull request
The following significant (semver-major) changes have been made since the previous Node v5.0.0 release.
- Buffer
- Cluster
- Worker emitted as first argument in 'message' event #5361.
- Crypto
- Dependencies
- DNS
- Add resolvePtr API to query plain DNS PTR records #4921.
- Domains
- Clear stack when no error handler #4659.
- File System
- The
fs.realpath()andfs.realpathSync()methods have been updated to use a more efficient libuv implementation. This change includes the removal of thecacheargument and the method can throw new errors #3594 - FS apis can now accept and return paths as Buffers #5616.
- Error handling and type checking improvements #5616, #5590, #4518, #3917.
- fs.read's string interface is deprecated #4525
- The
- HTTP
- 'clientError' can now be used to return custom errors from an HTTP server #4557.
- Modules
- Net
- OS X
- MACOSX_DEPLOYMENT_TARGET has been bumped up to 10.7 #6402.
- Path
- Improved type checking #5348.
- Process
- Readline
- Emit key info unconditionally #6024
- REPL
- Assignment to
_will emit a warning. #5535
- Assignment to
- Timers
- Fail early when callback is not a function #4362
- TLS
- TTY
- Previously deprecated setRawMode wrapper is removed #2528.
- Util
- Changes to Error object formatting #4582.
- Windows