Skip to content

Conversation

jasongin
Copy link
Member

Unlike most N-API functions, napi_get_last_error_info() should not clear the last error code when successful, because a pointer to (not a copy of) the error info structure is returned via an out parameter.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

n-api

Unlike most N-API functions, `napi_get_last_error_info()` should not
clear the last error code when successful, because a pointer to (not
a copy of) the error info structure is returned via an out parameter.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API. labels May 18, 2017
jasongin added a commit to nodejs/node-addon-api that referenced this pull request May 18, 2017
When a call to an N-API function caused an error for some reason other
than a JS exception, the fallback error message "Error in native
callback" was always reported because of incorrect logic in
`Napi::Error::New()`.

Then that fix exposed a bug in `napi_get_last_error_info()`, which I
have fixed here and also at nodejs/node#13087
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@mhdawson
Copy link
Member

@mhdawson
Copy link
Member

CI good, landing.

@mhdawson
Copy link
Member

Landed as a63b245

@mhdawson mhdawson closed this May 19, 2017
mhdawson pushed a commit that referenced this pull request May 19, 2017
Unlike most N-API functions, `napi_get_last_error_info()` should not
clear the last error code when successful, because a pointer to (not
a copy of) the error info structure is returned via an out parameter.

PR-URL: #13087
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@jasnell jasnell mentioned this pull request May 28, 2017
jasongin added a commit to nodejs/node-addon-api that referenced this pull request Jun 6, 2017
When a call to an N-API function caused an error for some reason other
than a JS exception, the fallback error message "Error in native
callback" was always reported because of incorrect logic in
`Napi::Error::New()`.

Then that fix exposed a bug in `napi_get_last_error_info()`, which I
have fixed here and also at nodejs/node#13087
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 10, 2018
Unlike most N-API functions, `napi_get_last_error_info()` should not
clear the last error code when successful, because a pointer to (not
a copy of) the error info structure is returned via an out parameter.

PR-URL: nodejs#13087
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 16, 2018
Unlike most N-API functions, `napi_get_last_error_info()` should not
clear the last error code when successful, because a pointer to (not
a copy of) the error info structure is returned via an out parameter.

Backport-PR-URL: #19447
PR-URL: #13087
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 16, 2018
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
When a call to an N-API function caused an error for some reason other
than a JS exception, the fallback error message "Error in native
callback" was always reported because of incorrect logic in
`Napi::Error::New()`.

Then that fix exposed a bug in `napi_get_last_error_info()`, which I
have fixed here and also at nodejs/node#13087
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
When a call to an N-API function caused an error for some reason other
than a JS exception, the fallback error message "Error in native
callback" was always reported because of incorrect logic in
`Napi::Error::New()`.

Then that fix exposed a bug in `napi_get_last_error_info()`, which I
have fixed here and also at nodejs/node#13087
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
When a call to an N-API function caused an error for some reason other
than a JS exception, the fallback error message "Error in native
callback" was always reported because of incorrect logic in
`Napi::Error::New()`.

Then that fix exposed a bug in `napi_get_last_error_info()`, which I
have fixed here and also at nodejs/node#13087
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
When a call to an N-API function caused an error for some reason other
than a JS exception, the fallback error message "Error in native
callback" was always reported because of incorrect logic in
`Napi::Error::New()`.

Then that fix exposed a bug in `napi_get_last_error_info()`, which I
have fixed here and also at nodejs/node#13087
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants