GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — node-api: add status napi_cannot_run_js by gabrielschulhof · Pull Request #47986 · nodejs/node

Conversation

@gabrielschulhof

@gabrielschulhof gabrielschulhof commented May 13, 2023

Copy link
Copy Markdown
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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@gabrielschulhof
gabrielschulhof requested review from legendecas and mhdawson May 13, 2023 08:21
@nodejs-github-bot

nodejs-github-bot commented May 13, 2023 •
edited by gabrielschulhof
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

Review requested:

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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 involving general changes in the lib/ or src/ directories. 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 Node-API. label May 13, 2023
@gabrielschulhof
node-api: add status napi_cannot_run_js
0e89301 
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
gabrielschulhof commented May 16, 2023
View reviewed changes
Comment thread doc/api/n-api.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@gabrielschulhof
doc: update doc/api/n-api.md
86277c9
legendecas
legendecas reviewed May 16, 2023
View reviewed changes
Comment thread test/node-api/test_cannot_run_js/test.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/node-api/test_cannot_run_js/binding.gyp Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/node-api/test_cannot_run_js/test_cannot_run_js.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
gabrielschulhof and others added 3 commits May 18, 2023 07:22
@gabrielschulhof @legendecas
Update test/node-api/test_cannot_run_js/test.js
fa45e0a 
Co-authored-by: Chengzhong Wu <[email protected]>
@gabrielschulhof
test: access global property to illicit error status
95137f5
@gabrielschulhof
heed the linter
72980e2
@gabrielschulhof

gabrielschulhof commented May 18, 2023

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
legendecas
legendecas approved these changes May 19, 2023
View reviewed changes

@legendecas legendecas left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM % nits.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/js-native-api/test_cannot_run_js/test.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@gabrielschulhof
test: address nit
d6f3232
@gabrielschulhof gabrielschulhof removed the needs-ci PRs that need a full CI run. label May 19, 2023
@nodejs-github-bot

nodejs-github-bot commented May 19, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/51881/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gabrielschulhof
test: use variables instead of target_defaults
d8bee9e 
Some platforms do not seem to support target_defaults.
@nodejs-github-bot

nodejs-github-bot commented May 19, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/51884/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gabrielschulhof
test: explicitly copy dependency
ab54044
@nodejs-github-bot

nodejs-github-bot commented May 19, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/51885/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request May 20, 2023
CI Reliability 2023-05-20 nodejs/reliability#571
Open
19 tasks
@gabrielschulhof
test: create job dependency to copy the common file
128a6f8
@nodejs-github-bot

nodejs-github-bot commented May 20, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/51889/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gabrielschulhof added a commit that referenced this pull request May 20, 2023
@gabrielschulhof
node-api: add status napi_cannot_run_js
19fa9f1 
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]>
@github-actions github-actions Bot mentioned this pull request May 22, 2023
CI Reliability 2023-05-22 nodejs/reliability#573
Open
16 tasks
This was referenced May 23, 2023
CI Reliability 2023-05-23 nodejs/reliability#574
Open
CI Reliability 2023-05-24 nodejs/reliability#575
Open
@legendecas legendecas mentioned this pull request May 24, 2023
node-api: define version 9 #48151
Merged
@github-actions github-actions Bot mentioned this pull request May 25, 2023
CI Reliability 2023-05-25 nodejs/reliability#576
Open
21 tasks
@kvakil kvakil mentioned this pull request May 25, 2023
Flaky js-native-api/test_cannot_run_js/test #48180
Closed
kvakil added a commit to kvakil/node that referenced this pull request May 25, 2023
@kvakil
test: mark test_cannot_run_js as flaky
6ee8586 
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
@kvakil kvakil mentioned this pull request May 25, 2023
test: mark test_cannot_run_js as flaky #48181
Merged
This was referenced May 26, 2023
CI Reliability 2023-05-26 nodejs/reliability#577
Open
CI Reliability 2023-05-27 nodejs/reliability#578
Open
nodejs-github-bot pushed a commit that referenced this pull request May 27, 2023
@kvakil
test: mark test_cannot_run_js as flaky
186307a 
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]>
@github-actions github-actions Bot mentioned this pull request May 28, 2023
CI Reliability 2023-05-28 nodejs/reliability#579
Open
20 tasks
targos pushed a commit that referenced this pull request May 30, 2023
@gabrielschulhof @targos
node-api: add status napi_cannot_run_js
e834979 
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
@kvakil @targos
test: mark test_cannot_run_js as flaky
c1c4796 
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
v20.3.0 release proposal #48332
Merged
@gabrielschulhof
gabrielschulhof deleted the cannot-call-into-js-status branch June 5, 2023 02:21
This was referenced Jun 21, 2023
Introduce status napi_cannot_call_into_js #30327
Closed
Handle the case where one cannot execute JavaScript #37446
Closed
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
@gabrielschulhof @danielleadams
node-api: add status napi_cannot_run_js
a66b2d7 
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
@kvakil @danielleadams
test: mark test_cannot_run_js as flaky
34b611f 
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
@gabrielschulhof @MoLow
node-api: add status napi_cannot_run_js
53c02b2 
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
@kvakil @MoLow
test: mark test_cannot_run_js as flaky
1847669 
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]>
@danielleadams danielleadams mentioned this pull request Jul 10, 2023
v18.17.0 release proposal #48694
Merged
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@kvakil @Ceres6
test: mark test_cannot_run_js as flaky
904ac33 
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
@kvakil @Ceres6
test: mark test_cannot_run_js as flaky
6dda2b1 
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

Reviewers

@legendecas legendecas legendecas approved these changes
@mhdawson mhdawson Awaiting requested review from mhdawson

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. node-api Issues and PRs related to Node-API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@gabrielschulhof @nodejs-github-bot @legendecas
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.