Skip to content

Conversation

@vmoroz
Copy link
Member

@vmoroz vmoroz commented Jul 7, 2025

Lately the stack overflow JSI test that runs JSI implementation for Node-API was failing.
The initial fix in PR #221 introduced a crash where in the isInstanceOf method we take a value from the vm::CallResult<bool> even if it has an error.
The crash became visible when started to use Clang compiler.
The more detailed analysis had shown that JSI for Node-API error reporting is significantly different from the base JSI implementation for Hermes. We must no use the isInstanceOf method at all.

In this PR we:

  • implement the throwJSException so that it follows the JSI for Hermes implementation;
  • fix the isInstanceOf method.

As a result the stack overflow test is passing.

@vmoroz vmoroz requested a review from a team as a code owner July 7, 2025 23:43
@vmoroz
Copy link
Member Author

vmoroz commented Jul 8, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vmoroz vmoroz enabled auto-merge (squash) July 8, 2025 01:12
@vmoroz vmoroz merged commit 4951c3c into microsoft:main Jul 8, 2025
12 checks passed
@vmoroz vmoroz deleted the PR/fix_error_reporting_in_JSI_for_Node-API branch July 8, 2025 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants