Skip to content

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Sep 27, 2025

Error handling logic was flawed in StatementExecutionHelper methods.

When IsNothing() is true or ToLocal() fails, return an empty MaybeLocal rather than creating an object or returning undefined.

Also, avoid use of ToLocalChecked

/cc @0hmX ... just fyi

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem. labels Sep 27, 2025
@jasnell jasnell force-pushed the jasnell/correct-error-handling-sqlite branch from ca54aac to 515711d Compare September 27, 2025 22:07
@nodejs-github-bot

This comment was marked as outdated.

This comment was marked as outdated.

@jasnell jasnell force-pushed the jasnell/correct-error-handling-sqlite branch from 515711d to 7b69c4f Compare September 28, 2025 00:05
@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2025
@nodejs-github-bot

This comment was marked as outdated.

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 28, 2025
@nodejs-github-bot

This comment was marked as outdated.

@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2025
@nodejs-github-bot

This comment was marked as outdated.

@addaleax
Copy link
Member

@jasnell Can you rebase this against main?

@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 30, 2025
@nodejs-github-bot

This comment was marked as outdated.

Error handling logic was flawed in StatementExecutionHelper
methods.
@jasnell jasnell force-pushed the jasnell/correct-error-handling-sqlite branch from 7b69c4f to 7c1aac3 Compare October 4, 2025 03:07
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 4, 2025

jasnell added a commit that referenced this pull request Oct 4, 2025
Error handling logic was flawed in StatementExecutionHelper
methods.

PR-URL: #60040
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Oct 4, 2025

Landed in 2869c12

@jasnell jasnell closed this Oct 4, 2025
targos pushed a commit that referenced this pull request Oct 6, 2025
Error handling logic was flawed in StatementExecutionHelper
methods.

PR-URL: #60040
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
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++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants