From 27925c4086d52e3858cbfabf0e55f52c20bce8ac Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 15 Jan 2018 02:51:36 +0800 Subject: doc: suggest not to throw JS errors from C++ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also provide an example on how to use internal/errors to handle errors in C++. PR-URL: https://github.com/nodejs/node/pull/18149 Refs: https://github.com/nodejs/node/issues/18106 Reviewed-By: Michaël Zasso Reviewed-By: Jon Moss Reviewed-By: Weijia Wang Reviewed-By: Daniel Bevenius Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson --- CPP_STYLE_GUIDE.md | 66 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 6 deletions(-) (limited to 'CPP_STYLE_GUIDE.md') diff --git a/CPP_STYLE_GUIDE.md b/CPP_STYLE_GUIDE.md index 6266ee03b7..5a275094ad 100644 --- a/CPP_STYLE_GUIDE.md +++ b/CPP_STYLE_GUIDE.md @@ -20,7 +20,8 @@ * [Others](#others) * [Type casting](#type-casting) * [Do not include `*.h` if `*-inl.h` has already been included](#do-not-include-h-if--inlh-has-already-been-included) - * [Avoid throwing JavaScript errors in nested C++ methods](#avoid-throwing-javascript-errors-in-nested-c-methods) + * [Avoid throwing JavaScript errors in C++ methods](#avoid-throwing-javascript-errors-in-c) + * [Avoid throwing JavaScript errors in nested C++ methods](#avoid-throwing-javascript-errors-in-nested-c-methods) Unfortunately, the C++ linter (based on [Google’s `cpplint`](https://github.com/google/styleguide)), which can be run @@ -213,12 +214,65 @@ instead of #include "util-inl.h" ``` -## Avoid throwing JavaScript errors in nested C++ methods +## Avoid throwing JavaScript errors in C++ -If you need to throw JavaScript errors from a C++ binding method, try to do it -at the top level and not inside of nested calls. +When there is a need to throw errors from a C++ binding method, try to +return the data necessary for constructing the errors to JavaScript, +then construct and throw the errors [using `lib/internal/errors.js`][errors]. -A lot of code inside Node.js is written so that typechecking etc. is performed -in JavaScript. +Note that in general, type-checks on arguments should be done in JavaScript +before the arguments are passed into C++. Then in the C++ binding, simply using +`CHECK` assertions to guard against invalid arguments should be enough. + +If the return value of the binding cannot be used to signal failures or return +the necessary data for constructing errors in JavaScript, pass a context object +to the binding and put the necessary data inside in C++. For example: + +```cpp +void Foo(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + // Let the JavaScript handle the actual type-checking, + // only assertions are placed in C++ + CHECK_EQ(args.Length(), 2); + CHECK(args[0]->IsString()); + CHECK(args[1]->IsObject()); + + int err = DoSomethingWith(args[0].As()); + if (err) { + // Put the data inside the error context + Local ctx = args[1].As(); + Local key = FIXED_ONE_BYTE_STRING(env->isolate(), "code"); + ctx->Set(env->context(), key, err).FromJust(); + } else { + args.GetReturnValue().Set(something_to_return); + } +} + +// In the initialize function +env->SetMethod(target, "foo", Foo); +``` + +```js +exports.foo = function(str) { + // Prefer doing the type-checks in JavaScript + if (typeof str !== 'string') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'str', 'string'); + } + + const ctx = {}; + const result = binding.foo(str, ctx); + if (ctx.code !== undefined) { + throw new errors.Error('ERR_ERROR_NAME', ctx.code); + } + return result; +}; +``` + +### Avoid throwing JavaScript errors in nested C++ methods + +When you have to throw the errors from C++, try to do it at the top level and +not inside of nested calls. Using C++ `throw` is not allowed. + +[errors]: https://github.com/nodejs/node/blob/master/doc/guides/using-internal-errors.md -- cgit v1.2.3