Skip to content

Conversation

gabrielschulhof
Copy link
Contributor

Add the new status in order to distinguish a state wherein an exception is pending from one wherein the engine is unable to execute JS. We take advantage of the new runtime add-on version reporting in order to remain forward compatible with add-ons that do not expect the new status code.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 13, 2023

Review requested:

  • @nodejs/gyp
  • @nodejs/node-api

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 13, 2023
@gabrielschulhof gabrielschulhof force-pushed the cannot-call-into-js-status branch from b2fd523 to a843857 Compare May 13, 2023 08:26
@gabrielschulhof gabrielschulhof added the node-api Issues and PRs related to the Node-API. label May 13, 2023
Add the new status in order to distinguish a state wherein an exception
is pending from one wherein the engine is unable to execute JS. We take
advantage of the new runtime add-on version reporting in order to remain
forward compatible with add-ons that do not expect the new status code.
@gabrielschulhof gabrielschulhof force-pushed the cannot-call-into-js-status branch from a843857 to 0e89301 Compare May 13, 2023 08:37
@tniessen tniessen changed the title node-api: Add status napi_cannot_run_js node-api: add status napi_cannot_run_js May 13, 2023
@gabrielschulhof
Copy link
Contributor Author

@legendecas I have addressed your review comments. Please take another look!

Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

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

LGTM % nits.

@gabrielschulhof gabrielschulhof removed the needs-ci PRs that need a full CI run. label May 19, 2023
@nodejs-github-bot
Copy link
Collaborator

Some platforms do not seem to support target_defaults.
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

gabrielschulhof added a commit that referenced this pull request May 20, 2023
Add the new status in order to distinguish a state wherein an exception
is pending from one wherein the engine is unable to execute JS. We take
advantage of the new runtime add-on version reporting in order to remain
forward compatible with add-ons that do not expect the new status code.

PR-URL: #47986
Reviewed-By: Chengzhong Wu <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
fasenderos pushed a commit to fasenderos/node that referenced this pull request May 22, 2023
Add the new status in order to distinguish a state wherein an exception
is pending from one wherein the engine is unable to execute JS. We take
advantage of the new runtime add-on version reporting in order to remain
forward compatible with add-ons that do not expect the new status code.

PR-URL: nodejs#47986
Reviewed-By: Chengzhong Wu <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
fasenderos pushed a commit to fasenderos/node that referenced this pull request May 22, 2023
Add the new status in order to distinguish a state wherein an exception
is pending from one wherein the engine is unable to execute JS. We take
advantage of the new runtime add-on version reporting in order to remain
forward compatible with add-ons that do not expect the new status code.

PR-URL: nodejs#47986
Reviewed-By: Chengzhong Wu <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
kvakil added a commit to kvakil/node that referenced this pull request May 25, 2023
This test has been failing occasionally since it was introduced ~5 days
ago. It was the nodejs#1 failing JS test in the most recent reliability
report. Mark it as flaky.

Fixes: nodejs#48180
Refs: nodejs#47986
Refs: nodejs/reliability#576
kvakil added a commit to kvakil/node that referenced this pull request May 25, 2023
This test has been failing occasionally since it was introduced ~5 days
ago. It was the most common failing JS test in the most recent
reliability report. Mark it as flaky.

Fixes: nodejs#48180
Refs: nodejs#47986
Refs: nodejs/reliability#576
nodejs-github-bot pushed a commit that referenced this pull request May 27, 2023
This test has been failing occasionally since it was introduced ~5 days
ago. It was the most common failing JS test in the most recent
reliability report. Mark it as flaky.

Fixes: #48180
Refs: #47986
Refs: nodejs/reliability#576
PR-URL: #48181
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit that referenced this pull request May 30, 2023
Add the new status in order to distinguish a state wherein an exception
is pending from one wherein the engine is unable to execute JS. We take
advantage of the new runtime add-on version reporting in order to remain
forward compatible with add-ons that do not expect the new status code.

PR-URL: #47986
Reviewed-By: Chengzhong Wu <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
targos pushed a commit that referenced this pull request May 30, 2023
This test has been failing occasionally since it was introduced ~5 days
ago. It was the most common failing JS test in the most recent
reliability report. Mark it as flaky.

Fixes: #48180
Refs: #47986
Refs: nodejs/reliability#576
PR-URL: #48181
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@gabrielschulhof gabrielschulhof restored the cannot-call-into-js-status branch June 3, 2023 18:10
@targos targos mentioned this pull request Jun 4, 2023
@gabrielschulhof gabrielschulhof deleted the cannot-call-into-js-status branch June 5, 2023 02:21
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
Add the new status in order to distinguish a state wherein an exception
is pending from one wherein the engine is unable to execute JS. We take
advantage of the new runtime add-on version reporting in order to remain
forward compatible with add-ons that do not expect the new status code.

PR-URL: #47986
Reviewed-By: Chengzhong Wu <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
This test has been failing occasionally since it was introduced ~5 days
ago. It was the most common failing JS test in the most recent
reliability report. Mark it as flaky.

Fixes: #48180
Refs: #47986
Refs: nodejs/reliability#576
PR-URL: #48181
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
Add the new status in order to distinguish a state wherein an exception
is pending from one wherein the engine is unable to execute JS. We take
advantage of the new runtime add-on version reporting in order to remain
forward compatible with add-ons that do not expect the new status code.

PR-URL: nodejs#47986
Reviewed-By: Chengzhong Wu <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
This test has been failing occasionally since it was introduced ~5 days
ago. It was the most common failing JS test in the most recent
reliability report. Mark it as flaky.

Fixes: nodejs#48180
Refs: nodejs#47986
Refs: nodejs/reliability#576
PR-URL: nodejs#48181
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
This test has been failing occasionally since it was introduced ~5 days
ago. It was the most common failing JS test in the most recent
reliability report. Mark it as flaky.

Fixes: nodejs#48180
Refs: nodejs#47986
Refs: nodejs/reliability#576
PR-URL: nodejs#48181
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
This test has been failing occasionally since it was introduced ~5 days
ago. It was the most common failing JS test in the most recent
reliability report. Mark it as flaky.

Fixes: nodejs#48180
Refs: nodejs#47986
Refs: nodejs/reliability#576
PR-URL: nodejs#48181
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@gabrielschulhof gabrielschulhof restored the cannot-call-into-js-status branch December 30, 2024 19:45
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++. lib / src Issues and PRs related to general changes in the lib or src directory. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants