Include extended error codes in error messages

[zachlefevre]

SQLITE error logs should include that error's extended error code.

[ask-bonk]

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

This PR adds the SQLite extended error code to error messages in dbErrorMessage(). A few issues:

1. [MEDIUM] Extended error code is appended as a raw integer. sqlite3_extended_errcode() returns an int — appending it directly produces something like SQLITE_CONSTRAINT- 19 (or SQLITE_CONSTRAINT- 2067 for SQLITE_CONSTRAINT_UNIQUE). The raw number isn't useful for debugging. Consider using sqlite3_errstr() which accepts extended error codes and returns a human-readable string (e.g., "UNIQUE constraint failed").

2. [LOW] Missing space before dash. The format string "- " produces SQLITE_ERROR- 1 — there should be a space before the dash for readability.

3. [LOW] Variable naming. extendedErrorMessage is misleading since sqlite3_extended_errcode() returns an int, not a message string. Something like extendedErrorCode would be more accurate.

These comments were generated by an AI code review assistant and may not be perfect.

[ask-bonk]

A few issues with these lines:

1. sqlite3_extended_errcode() returns an int, but the variable is named extendedErrorMessage — this is misleading.
2. The extended error code is appended as a raw integer, which isn't very useful for debugging. The codebase already uses sqlite3_errstr() elsewhere (e.g., in SQLITE_CALL_NODB) which handles extended codes and returns a human-readable string.
3. There's a missing space before the dash: "- " should be " - " (or use " (extended: ...)" to clearly delineate it).
4. When there is no extended info, sqlite3_extended_errcode() returns the same value as the primary code, so you'd get redundant output like SQLITE_ERROR - SQLITE_ERROR. Consider only appending when the extended code differs from the primary code.

Suggested change

	auto extendedErrorMessage = sqlite3_extended_errcode(db);
	if (int offset = sqlite3_error_offset(db); offset != -1) {
	msg = kj::strTree(kj::mv(msg), " at offset ", offset);
	}
	msg = kj::strTree(kj::mv(msg), ": ", namedErrorCode(errorCode));
	msg = kj::strTree(kj::mv(msg), ": ", namedErrorCode(errorCode), "- ", extendedErrorMessage);
	auto extendedErrorCode = sqlite3_extended_errcode(db);
	if (int offset = sqlite3_error_offset(db); offset != -1) {
	msg = kj::strTree(kj::mv(msg), " at offset ", offset);
	}
	msg = kj::strTree(kj::mv(msg), ": ", namedErrorCode(errorCode));
	if (extendedErrorCode != errorCode) {
	msg = kj::strTree(kj::mv(msg), " (extended: ", sqlite3_errstr(extendedErrorCode), ")");
	}

[jasnell]

Good review, Bonk.

[ask-bonk]

Review posted successfully on PR #6321. Here's a summary of the findings:

1. [MEDIUM] Raw integer not useful for debugging — sqlite3_extended_errcode() returns an int that gets appended as-is (e.g., 2067). Suggested using sqlite3_errstr() instead, which returns a human-readable description and is already used elsewhere in the file.

2. [LOW] Missing space before dash — "- " should be " - " for readability.

3. [LOW] Misleading variable name — extendedErrorMessage holds an int, not a message. Suggested extendedErrorCode.

I also included a concrete suggestion that only appends the extended code when it differs from the primary code (to avoid redundant output like SQLITE_ERROR - SQLITE_ERROR).

github run