Record the cause of an OPFSCoopSyncVFS open that failed asynchronously - #357
Open
lalexdotcom wants to merge 2 commits into
Open
lalexdotcom wants to merge 2 commits into
lalexdotcom wants to merge 2 commits into
Conversation
jOpen starts the work for a main database, returns SQLITE_BUSY and answers the retried call. When the asynchronous phase failed, its error went to the console only: the retried call returns SQLITE_CANTOPEN from a branch that has no error of its own, so a caller cannot tell a file held by another context from a file that is not there. Every other error return of this VFS assigns lastError before returning. This was the only one that did not, which also made it the only path where xGetLastError could report an error left by an earlier call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The test holds the database file from a worker of its own and drives jOpen directly rather than through sqlite3_open_v2, which is the caller's situation: there is no connection to ask sqlite3_errmsg. It needs the test worker's VFS proxy to pass plain properties through. The proxy returns only functions, so a test can call VFS methods but cannot read the state a call leaves behind; every other property answers undefined. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OPFSCoopSyncVFScannot open a database synchronously:jOpenstarts the work, returnsSQLITE_BUSY, and answers the retried call. When that asynchronous phase fails, the error goes to the console and nowhere else — the retriedjOpenreturnsSQLITE_CANTOPENfrom a branch that has no error of its own, andlastErrorstill holds whatever it held before, if anything.The caller is then told only that the database could not be opened. It cannot tell a file held by another context — an ordinary, retryable condition — from a file that is not there.
One line changes:
this.lastError = ein that catch.Why this branch, and only this one
Every other error return of this VFS records its cause first:
jOpen's synchronous catch,jDelete,jAccess,jClose,jRead,jWrite,jTruncate,jSync,jFileSizeandjFileControlall assignlastErrorbefore returning theirSQLITE_IOERR_*orSQLITE_CANTOPEN. The non-OK returns that do not assign it are results rather than failures —SQLITE_BUSYfromjOpenandjLock,SQLITE_IOERR_SHORT_READ, andjFileControl'sSQLITE_NOTFOUND.So the asynchronous open is the single error path that reports nothing. It is also, for the same reason, the only path where
xGetLastErrorcan serve a stale message: nothing ever clearslastError, so aSQLITE_CANTOPENraised here is reported with whatever error an earlier call happened to leave.What it looks like
A worker holds an exclusive
FileSystemSyncAccessHandleondemo— a connection elsewhere, or a worker that has just been terminated and whose handles the engine has not reclaimed yet:With the change,
lastErrorholds theNoModificationAllowedError.What it does not change, measured rather than assumed: SQLite does not fold
xGetLastErrorinto the connection's message here.sqlite3.open_v2on that held file rejects withunable to open database fileboth before and after — the generic string forSQLITE_CANTOPEN. So the VFS instance is the only place the cause exists, which is precisely why it has to be recorded there.The test
test/vfs_open_last_error.js, wired intotest/OPFSCoopSyncVFS.test.js. It holds the file from a worker of its own, then drivesjOpendirectly and asserts the recorded cause. Directly, because that is where the distinction lives: since #330 a failedsqlite3_open_v2does report the connection's message, but that message isunable to open database filewhatever the VFS knows.It fails on master on the
defaultandasyncifybuilds (Expected null to be truthy) and passes with the change. The file's other tests are unaffected.It needs one line in
test/test-worker.js. The VFS proxy returns only functions, so a test can call VFS methods but cannot read the state a call leaves behind; every non-function property answersundefinedtoday,lastErrorincluded. Passing plain properties through makes it observable and changes nothing else.Note
Written with Claude Code; the commits are co-signed.
Checklist
non-exclusive, royalty-free, irrevocable copyright license to reproduce, prepare
derivative works of, publicly display, sublicense, and distribute this
Contribution and such derivative works.
Contribution contains no content requiring a license from any third party.