Report Extended Code

[zachlefevre]

No description provided.

[ask-bonk]

I'm Bonk, and I've done a quick review of your PR.

This PR adds extended SQLite error code reporting to SqliteObserver::reportQueryEvent and QueryEvent.

Issues found (ranked by severity):

1. [HIGH] setQueryExtendedCode assigns to the wrong member — writes queryResult instead of extendedErrorCode.
2. [HIGH] The existing test in sqlite-test.c++ overrides reportQueryEvent with the old signature (missing the new extendedErrorCode parameter). Since the override uses override, this will be a compile error on the new base class signature. The test needs to be updated.

---

This review was written by an AI assistant and may not be perfect. Please verify all suggestions before applying.

[ask-bonk]

I'm Bonk, and I've done a quick review of your PR.

This PR adds extended SQLite error code reporting to SqliteObserver::reportQueryEvent and QueryEvent.

Issues found (ranked by severity):

1. [HIGH] setQueryExtendedCode at src/workerd/util/sqlite.h:622 assigns to queryResult instead of extendedErrorCode — a copy-paste bug that silently drops the extended code and clobbers the query result.
2. [HIGH] The existing test override of reportQueryEvent in src/workerd/util/sqlite-test.c++:923 still has the old signature (missing the new extendedErrorCode parameter). Since it uses override, this will be a compile error.

Both comments with a suggested fix have been posted to the PR.

github run

[codspeed-hq]

Merging this PR will improve performance by 40.18%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 71 untouched benchmarks
⏩ 129 skipped benchmarks¹

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
⚡	simpleStringBody[Response]	27.2 µs	19.4 µs	+40.18%

---

_{Comparing zlefevre/report_extended_code (ccf5da4) with main (22c1ca0)}

Footnotes

1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[ask-bonk]

Correction to my earlier review: both issues I flagged were false positives — apologies for the noise.

1. setQueryExtendedCode correctly assigns to extendedErrorCode, not queryResult. I misread the diff.
2. The test override in sqlite-test.c++ already includes the new int extendedErrorCode parameter.

The changes look correct. LGTM.

[ask-bonk]

LGTM

github run