Skip to content

Add StoreError and StoreResult - #37

Open
sidracha wants to merge 1 commit into
srachaba-2-sqlite-storefrom
srachaba-3-store-errors
Open

sidracha wants to merge 1 commit into
srachaba-2-sqlite-storefrom
srachaba-3-store-errors

Conversation

@sidracha

Copy link
Copy Markdown
Contributor

Example new error message/handling on database read error:

sjj cc init --server http://localhost:8080

Error: Failed to initialize workspace: Backend(BackendInitError(Status { code: Internal, message: "storage write error: attempt to write a readonly database", metadata: MetadataMap { headers: {"content-type": "application/grpc", "date": "Mon, 31 Aug 2026 06:35:36 GMT", "content-length": "0"} }, source: None }))

A large amount of the diffs are simply changing the return type to StoreResult or adding '?' to await

@sidracha
sidracha force-pushed the srachaba-3-store-errors branch from c520eec to c69a690 Compare August 31, 2026 07:03
@sidracha
sidracha force-pushed the srachaba-3-store-errors branch from c69a690 to d8ea8da Compare August 31, 2026 18:45
@sidracha
sidracha force-pushed the srachaba-3-store-errors branch from d8ea8da to 65b7b66 Compare August 31, 2026 19:08
@sidracha
sidracha force-pushed the srachaba-3-store-errors branch from 65b7b66 to e9e5cfa Compare September 1, 2026 00:01
@sidracha
sidracha force-pushed the srachaba-3-store-errors branch 2 times, most recently from 34a6274 to 5f854a3 Compare September 1, 2026 22:44
@sidracha
sidracha force-pushed the srachaba-3-store-errors branch from 5f854a3 to 22f6fb8 Compare September 1, 2026 22:52
@sidracha
sidracha force-pushed the srachaba-3-store-errors branch from 22f6fb8 to 4ea82db Compare September 1, 2026 22:59

@nasamuffin nasamuffin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be great if you can add some test cases exercising these errors. doesn't do us much good if it turns out the errors themselves are broken in some way.

@sidracha
sidracha force-pushed the srachaba-3-store-errors branch from 4ea82db to 16c7e4a Compare September 1, 2026 23:59
@sidracha
sidracha force-pushed the srachaba-3-store-errors branch 2 times, most recently from 293bd19 to 9c9ecb7 Compare September 2, 2026 00:22
@sidracha
sidracha force-pushed the srachaba-3-store-errors branch from 9c9ecb7 to 039383f Compare September 2, 2026 00:47
@sidracha

sidracha commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

would be great if you can add some test cases exercising these errors. doesn't do us much good if it turns out the errors themselves are broken in some way.

Added unit tests as well as CLI integration tests testing that these errors work, and return the current message on each type of error (Write, Read, Encode, etc)

@sidracha
sidracha force-pushed the srachaba-3-store-errors branch from 039383f to d3ab5e8 Compare September 2, 2026 16:21
Store methods previously ignored database write failures, and read
failures returned NotFound on database reads instead of reporting a
database failure. Introduce 'StoreError' enum with different store error
types and 'StoreResult<T>'. These error types are converted into tonic
errors with messages, which are displayed on the cli. This follows the
same convention as the upstream jj-lib BackendResult/OpStoreResult.
Wires this result into the server backend and opstore files, using ? for an
early return on an error.

Add comprehensive unit tests as well as integration tests that test the
StoreErrors return the correct error and error messages. Unit tests in sqlitestore
cover all 4 error types: write failures on read-only SQLite instances, read failures
on missing tables, data corruption decode errors, and encode errors.
@sidracha
sidracha force-pushed the srachaba-3-store-errors branch from d3ab5e8 to 8cb4259 Compare September 2, 2026 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants