Skip to content

Commit b05f09a

Browse files
joyeecheungevanlucas
authored andcommitted
doc: suggest not to throw JS errors from C++
Also provide an example on how to use internal/errors to handle errors in C++. PR-URL: #18149 Refs: #18106 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent 1506eb5 commit b05f09a

File tree

1 file changed

+60
-6
lines changed

1 file changed

+60
-6
lines changed

CPP_STYLE_GUIDE.md

+60-6
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
* [Others](#others)
2121
* [Type casting](#type-casting)
2222
* [Do not include `*.h` if `*-inl.h` has already been included](#do-not-include-h-if--inlh-has-already-been-included)
23-
* [Avoid throwing JavaScript errors in nested C++ methods](#avoid-throwing-javascript-errors-in-nested-c-methods)
23+
* [Avoid throwing JavaScript errors in C++ methods](#avoid-throwing-javascript-errors-in-c)
24+
* [Avoid throwing JavaScript errors in nested C++ methods](#avoid-throwing-javascript-errors-in-nested-c-methods)
2425

2526
Unfortunately, the C++ linter (based on
2627
[Google’s `cpplint`](https://github.com/google/styleguide)), which can be run
@@ -213,12 +214,65 @@ instead of
213214
#include "util-inl.h"
214215
```
215216

216-
## Avoid throwing JavaScript errors in nested C++ methods
217+
## Avoid throwing JavaScript errors in C++
217218

218-
If you need to throw JavaScript errors from a C++ binding method, try to do it
219-
at the top level and not inside of nested calls.
219+
When there is a need to throw errors from a C++ binding method, try to
220+
return the data necessary for constructing the errors to JavaScript,
221+
then construct and throw the errors [using `lib/internal/errors.js`][errors].
220222

221-
A lot of code inside Node.js is written so that typechecking etc. is performed
222-
in JavaScript.
223+
Note that in general, type-checks on arguments should be done in JavaScript
224+
before the arguments are passed into C++. Then in the C++ binding, simply using
225+
`CHECK` assertions to guard against invalid arguments should be enough.
226+
227+
If the return value of the binding cannot be used to signal failures or return
228+
the necessary data for constructing errors in JavaScript, pass a context object
229+
to the binding and put the necessary data inside in C++. For example:
230+
231+
```cpp
232+
void Foo(const FunctionCallbackInfo<Value>& args) {
233+
Environment* env = Environment::GetCurrent(args);
234+
// Let the JavaScript handle the actual type-checking,
235+
// only assertions are placed in C++
236+
CHECK_EQ(args.Length(), 2);
237+
CHECK(args[0]->IsString());
238+
CHECK(args[1]->IsObject());
239+
240+
int err = DoSomethingWith(args[0].As<String>());
241+
if (err) {
242+
// Put the data inside the error context
243+
Local<Object> ctx = args[1].As<Object>();
244+
Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "code");
245+
ctx->Set(env->context(), key, err).FromJust();
246+
} else {
247+
args.GetReturnValue().Set(something_to_return);
248+
}
249+
}
250+
251+
// In the initialize function
252+
env->SetMethod(target, "foo", Foo);
253+
```
254+
255+
```js
256+
exports.foo = function(str) {
257+
// Prefer doing the type-checks in JavaScript
258+
if (typeof str !== 'string') {
259+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'str', 'string');
260+
}
261+
262+
const ctx = {};
263+
const result = binding.foo(str, ctx);
264+
if (ctx.code !== undefined) {
265+
throw new errors.Error('ERR_ERROR_NAME', ctx.code);
266+
}
267+
return result;
268+
};
269+
```
270+
271+
### Avoid throwing JavaScript errors in nested C++ methods
272+
273+
When you have to throw the errors from C++, try to do it at the top level and
274+
not inside of nested calls.
223275

224276
Using C++ `throw` is not allowed.
277+
278+
[errors]: https://github.com/nodejs/node/blob/master/doc/guides/using-internal-errors.md

0 commit comments

Comments
 (0)