Tolerate missing role on user - #388
Merged
Merged
Conversation
kriszyp
marked this pull request as ready for review
April 20, 2026 18:25
DavidCockerill
approved these changes
Apr 20, 2026
cb1kenobi
approved these changes
Apr 20, 2026
Devin-Holland
approved these changes
Apr 20, 2026
heskew
approved these changes
Apr 20, 2026
heskew
left a comment
heskew
left a comment
Member
There was a problem hiding this comment.
Seems correct, but not caught early because user is (often) any typed? User interface has the role as optional.
kriszyp
added a commit
that referenced
this pull request
Jun 20, 2026
…can skip it (#429) A truncated/incomplete UNCOMPRESSED blob file at the source threw a plain Error with no `.code`, so the sender forwarded errorCode=undefined and the receiver classified it transient — holding the replication resume cursor forever (the same wedge #403 fixed for ENOENT, but for a truncated blob instead of a missing one). Confirmed live on a 5.1.6 cluster: jjillCache copy stuck behind "Blob is incomplete" while ENOENT blobs correctly advanced past. Stamp ERR_BLOB_INCOMPLETE (via incompleteBlobError()) on the two writer-finished READ paths (checkCompletion and the streaming reader) so isPermanentSourceBlobErrorCode (harper-pro) advances the resume cursor past it; the diverged record is left for proactive backfill (#388/#418). Scoped to avoid two data-loss traps: - Only UNCOMPRESSED blobs are coded. The read paths compare the header's uncompressed `size` against raw on-disk bytes, so a COMPLETE compressed blob also trips the incomplete check — coding it would let replication skip + unlink a healthy blob. - The sync msgpackr-encode path has no writer-finished guard (a mismatch there can be a mid-write blob), so it stays uncoded -> receiver holds and retries. The read-timeout incompleteness also stays uncoded (transient). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kriszyp
added a commit
that referenced
this pull request
Jun 22, 2026
…can skip it (#429) A truncated/incomplete UNCOMPRESSED blob file at the source threw a plain Error with no `.code`, so the sender forwarded errorCode=undefined and the receiver classified it transient — holding the replication resume cursor forever (the same wedge #403 fixed for ENOENT, but for a truncated blob instead of a missing one). Confirmed live on a 5.1.6 cluster: jjillCache copy stuck behind "Blob is incomplete" while ENOENT blobs correctly advanced past. Stamp ERR_BLOB_INCOMPLETE (via incompleteBlobError()) on the two writer-finished READ paths (checkCompletion and the streaming reader) so isPermanentSourceBlobErrorCode (harper-pro) advances the resume cursor past it; the diverged record is left for proactive backfill (#388/#418). Scoped to avoid two data-loss traps: - Only UNCOMPRESSED blobs are coded. The read paths compare the header's uncompressed `size` against raw on-disk bytes, so a COMPLETE compressed blob also trips the incomplete check — coding it would let replication skip + unlink a healthy blob. - The sync msgpackr-encode path has no writer-finished guard (a mismatch there can be a mid-write blob), so it stays uncoded -> receiver holds and retries. The read-timeout incompleteness also stays uncoded (transient). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kriszyp
added a commit
that referenced
this pull request
Jun 23, 2026
…sweep (#1387) * feat(blob): add findIncompleteBlobRefs generator and isBlobComplete for repair sweep Exports three new functions used by the harper-pro blob repair operation: - `findIncompleteBlobRefs(database, databaseName?)` — async generator that walks the primary store looking for records with HAS_BLOBS set whose blob files are missing, truncated, or errored. Throttles via STORAGE_BLOBCLEANUPSPEED. - `isBlobComplete(blob)` — checks whether a FileBackedBlob's on-disk file is fully written (header size == file size, no ERROR_TYPE header). - Re-exports existing `findBlobsInObject` and `isSaving` for the repair layer. `isBlobComplete` reads only the 8-byte header (stat + openSync/readSync/closeSync) rather than the whole file, so it's safe to call on large blobs during a sweep. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(blob): add unit tests for isBlobComplete and findIncompleteBlobRefs Both exports were flagged by the automated review as untested. Tests cover: - isBlobComplete: native Blob → true, unsaved FileBackedBlob → false, fully saved → true, deleted file (ENOENT) → false - findIncompleteBlobRefs: yields record with missing blob file, skips record with complete blob Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(blob): wrap entry.value decode in decodeFromDatabase in findIncompleteBlobRefs encodeBlobsWithFilePath resets currentStore to undefined in its finally block. When findIncompleteBlobRefs subsequently accesses entry.value from a lazy getRange iterator, the blob unpack extension reads currentStore (undefined) and throws 'No store specified'. The catch block swallowed this silently, so records with incomplete blobs were never yielded. Fix: wrap the findBlobsInObject(entry.value, ...) call in decodeFromDatabase so currentStore is set to the table's rootStore before blob decoding. This is the same pattern used by receiveBlobs and other callers that decode blobs outside the normal encodeBlobsWithFilePath window. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: use entry.key (not entry.id) in findIncompleteBlobRefs and catch isBlobFileComplete errors lmdb-js getRange yields entries as {key, value, version}; `entry.id` does not exist, so `recordId: entry.id` was always undefined. Also adds inner try-catch around isBlobFileComplete so a path-construction error (non-ENOENT) marks the blob as incomplete rather than being silently swallowed by the outer catch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(blob): cover truncated/error-stub isBlobComplete and HAS_BLOBS skip in findIncompleteBlobRefs Extend coverage per PR review: isBlobComplete now also asserts the truncated (header size != file size) and error-stub (ERROR_TYPE header) cases; findIncompleteBlobRefs asserts a record without the HAS_BLOBS metadata flag is not yielded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(blob): correct isBlobFileComplete for compressed blobs isBlobFileComplete compared the header size field against fileSize - HEADER_SIZE. For a DEFLATE blob the header size is the *uncompressed* length (writeBlobWithStream stores deflate.bytesWritten, the input byte count), while the on-disk body is the *compressed* stream, so a correctly-saved compressed blob was wrongly reported incomplete. This made findIncompleteBlobRefs (the proactive repair sweep) flag every compressed blob as a false repair candidate. (Codex review on #1387.) For DEFLATE blobs, verify completeness by streaming the body through inflate and comparing the decompressed length to the header size; a truncated stream errors (Z_BUF_ERROR) or yields fewer bytes. Streaming (vs inflateSync on the whole buffer) keeps memory bounded during the sweep, which may touch many large blobs. The uncompressed path keeps the existing body-length check; an explicit UNKNOWN_SIZE placeholder check is added. isBlobFileComplete/isBlobComplete are now async to accommodate the streamed inflate; findIncompleteBlobRefs collects candidate blobs synchronously then awaits the checks. Tests: a correctly-saved compressed blob now reports complete (fails before this fix) and a truncated compressed blob still reports incomplete. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(blob): code the confidently-incomplete blob error so replication can skip it (#429) A truncated/incomplete UNCOMPRESSED blob file at the source threw a plain Error with no `.code`, so the sender forwarded errorCode=undefined and the receiver classified it transient — holding the replication resume cursor forever (the same wedge #403 fixed for ENOENT, but for a truncated blob instead of a missing one). Confirmed live on a 5.1.6 cluster: jjillCache copy stuck behind "Blob is incomplete" while ENOENT blobs correctly advanced past. Stamp ERR_BLOB_INCOMPLETE (via incompleteBlobError()) on the two writer-finished READ paths (checkCompletion and the streaming reader) so isPermanentSourceBlobErrorCode (harper-pro) advances the resume cursor past it; the diverged record is left for proactive backfill (#388/#418). Scoped to avoid two data-loss traps: - Only UNCOMPRESSED blobs are coded. The read paths compare the header's uncompressed `size` against raw on-disk bytes, so a COMPLETE compressed blob also trips the incomplete check — coding it would let replication skip + unlink a healthy blob. - The sync msgpackr-encode path has no writer-finished guard (a mismatch there can be a mid-write blob), so it stays uncoded -> receiver holds and retries. The read-timeout incompleteness also stays uncoded (transient). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(blob): make isBlobComplete async to drop Promise.resolve wrappers Addresses cb1kenobi review comment on #1387. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Kris Zyp <kris@harperdb.io>
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
5 participants
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.
No description provided.