Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add napi_fatal_error API #13971

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,23 @@ Returns `napi_ok` if the API succeeded.
This API returns true if an exception is pending.
### Fatal Errors
In the event of an unrecoverable error in a native module, a fatal error can be
thrown to immediately terminate the process.
#### napi_fatal_error
<!-- YAML
added: REPLACEME
-->
```C
NAPI_EXTERN NAPI_NO_RETURN void napi_fatal_error(const char* location, const char* message);
```

- `[in] location`: Optional location at which the error occurred.
- `[in] message`: The message associated with the error.

The function call does not return, the process will be terminated.

## Object Lifetime management

Expand Down
5 changes: 5 additions & 0 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,11 @@ napi_status napi_get_last_error_info(napi_env env,
return napi_ok;
}

NAPI_NO_RETURN void napi_fatal_error(const char* location,
const char* message) {
node::FatalError(location, message);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this require node internals?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but since it was already included in the file I assumed it wasn't a big deal. Since this is the v8-specific implementation of N-API, this can still be considered internal (I think).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I forgot this hadn't landed. #13892

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, ok, I hadn't seen that. It seems like it would be best then to adapt an implementation into node_api.cc that mimics the node::FatalError behavior?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is what I had expected we would need to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on discussion today in the WG meeting, we'll be reverting back to the original change and removing the duplicated code.

}

napi_status napi_create_function(napi_env env,
const char* utf8name,
napi_callback cb,
Expand Down
9 changes: 9 additions & 0 deletions src/node_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@
# define NAPI_MODULE_EXPORT __attribute__((visibility("default")))
#endif

#ifdef __GNUC__
#define NAPI_NO_RETURN __attribute__((noreturn))
#else
#define NAPI_NO_RETURN
#endif


typedef void (*napi_addon_register_func)(napi_env env,
napi_value exports,
Expand Down Expand Up @@ -104,6 +110,9 @@ NAPI_EXTERN napi_status
napi_get_last_error_info(napi_env env,
const napi_extended_error_info** result);

NAPI_EXTERN NAPI_NO_RETURN void napi_fatal_error(const char* location,
const char* message);

// Getters for defined singletons
NAPI_EXTERN napi_status napi_get_undefined(napi_env env, napi_value* result);
NAPI_EXTERN napi_status napi_get_null(napi_env env, napi_value* result);
Expand Down
8 changes: 8 additions & 0 deletions test/addons-napi/test_fatal/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"targets": [
{
"target_name": "test_fatal",
"sources": [ "test_fatal.c" ]
}
]
}
18 changes: 18 additions & 0 deletions test/addons-napi/test_fatal/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';
const common = require('../../common');
const assert = require('assert');
const child_process = require('child_process');
const test_fatal = require(`./build/${common.buildType}/test_fatal`);

// Test in a child process because the test code will trigger a fatal error
// that crashes the process.
if (process.argv[2] === 'child') {
test_fatal.Test();
return;
}

const p = child_process.spawnSync(
process.execPath, [ '--napi-modules', __filename, 'child' ]);
assert.ifError(p.error);
assert.ok(p.stderr.toString().includes(
'FATAL ERROR: test_fatal::Test fatal message'));
18 changes: 18 additions & 0 deletions test/addons-napi/test_fatal/test_fatal.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#include <node_api.h>
#include "../common.h"

napi_value Test(napi_env env, napi_callback_info info) {
napi_fatal_error("test_fatal::Test", "fatal message");
return NULL;
}

void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
napi_property_descriptor properties[] = {
DECLARE_NAPI_PROPERTY("Test", Test),
};

NAPI_CALL_RETURN_VOID(env, napi_define_properties(
env, exports, sizeof(properties) / sizeof(*properties), properties));
}

NAPI_MODULE(addon, Init)