Skip to content

sqlite: refactor error helpers and user function pointers - #62794

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
thisalihassan:sqlite-refactor-error-and-udf-ptrs
Aug 8, 2026
Merged

sqlite: refactor error helpers and user function pointers#62794
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
thisalihassan:sqlite-refactor-error-and-udf-ptrs

Conversation

@thisalihassan

@thisalihassan thisalihassan commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Two small independent refactors.

  1. Consolidate CreateSQLiteError overloads
  2. BaseObjectWeakPtr for user functions (style consistency)

@nodejs-github-bot

nodejs-github-bot commented Apr 17, 2026

Copy link
Copy Markdown
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 Apr 17, 2026
@codecov

codecov Bot commented Apr 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.74359% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.31%. Comparing base (27d6cfa) to head (43d0172).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 89.74% 1 Missing and 3 partials ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #62794   +/-   ##
=======================================
  Coverage   90.31%   90.31%           
=======================================
  Files         759      759           
  Lines      248258   248240   -18     
  Branches    46854    46853    -1     
=======================================
- Hits       224225   224209   -16     
+ Misses      15465    15462    -3     
- Partials     8568     8569    +1
Files with missing lines Coverage Δ
src/node_sqlite.h 82.60% <ø> (ø)
src/node_sqlite.cc 80.89% <89.74%> (+0.05%) ⬆️

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thisalihassan

thisalihassan commented Apr 21, 2026

Copy link
Copy Markdown
Contributor Author

@nodejs/sqlite @geeksilva97 can I get a review please?

@geeksilva97

geeksilva97 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Sure. Will take a look.

@geeksilva97

geeksilva97 commented May 6, 2026

Copy link
Copy Markdown
Contributor

cc @nodejs/cpp-reviewers

@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label May 6, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 6, 2026
@nodejs-github-bot

nodejs-github-bot commented May 6, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/73204/

@geeksilva97
geeksilva97 requested a review from addaleax May 7, 2026 18:29
@trivikr

trivikr commented Jul 25, 2026

Copy link
Copy Markdown
Member

Is this okay to land @geeksilva97, or do we need an additional review from CPP team?

@geeksilva97

geeksilva97 commented Jul 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Is this okay to land @geeksilva97, or do we need an additional review from CPP team?

PR LGTM. We can get it landed, I was just looking for another eye before moving further.

We need a new CI run since it's been a while

@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 26, 2026
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 26, 2026
@github-actions

This comment was marked as outdated.

Sign in to view
@thisalihassan

thisalihassan commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@geeksilva97 @trivikr do I need to rebase or something?

@geeksilva97

geeksilva97 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

@geeksilva97 @trivikr do I need to rebase or something?

yes, please

@thisalihassan
thisalihassan force-pushed the sqlite-refactor-error-and-udf-ptrs branch from 7d3237b to fe5a33c Compare July 27, 2026 08:44
@geeksilva97 geeksilva97 added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Jul 27, 2026
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 27, 2026
@github-actions

This comment was marked as outdated.

Sign in to view
@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Aug 5, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 5, 2026
@github-actions github-actions Bot added the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Aug 5, 2026
@github-actions

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@trivikr trivikr removed the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Aug 6, 2026
@nodejs-github-bot

nodejs-github-bot commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/75554/

@trivikr

This comment was marked as outdated.

Sign in to view
@thisalihassan

thisalihassan commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @trivikr let me check

@thisalihassan
thisalihassan force-pushed the sqlite-refactor-error-and-udf-ptrs branch from fe5a33c to 43d0172 Compare August 6, 2026 15:22
@thisalihassan

thisalihassan commented Aug 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@trivikr I have fixed the compliation issue, but the CI is red due to infra outages
cc: @geeksilva97

@trivikr

This comment was marked as outdated.

Sign in to view
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2026
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@trivikr trivikr removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2026
@nodejs-github-bot

nodejs-github-bot commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/75644/

@trivikr trivikr added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 8, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 8, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 347e266 into nodejs:main Aug 8, 2026
95 of 106 checks passed
@nodejs-github-bot

nodejs-github-bot commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Landed in 347e266

aduh95 pushed a commit that referenced this pull request Aug 13, 2026
@thisalihassan @aduh95
sqlite: refactor error helpers and user function pointers
55f8e7c 
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com>
PR-URL: #62794
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@geeksilva97 geeksilva97 geeksilva97 approved these changes
@addaleax addaleax Awaiting requested review from addaleax

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@thisalihassan @nodejs-github-bot @geeksilva97 @trivikr @jasnell