Repository navigation
feat(attachments): audit attachments after each sync and report missing ones to Sentry - #90
Draft
gmaclennan wants to merge 2 commits into
Draft
gmaclennan wants to merge 2 commits into
gmaclennan wants to merge 2 commits into
Conversation
… watches added projects
…ng ones to Sentry
This was referenced Nov 5, 2026
Partly synced attachments are served as 500, truncated 200 or 404 with no way to tell them apart
#81
Open
gmaclennan
commented
Nov 5, 2026
gmaclennan
left a comment
Member
Author
There was a problem hiding this comment.
Really not sure about this approach, while useful it feels like a compromise of user privacy and it adds a lot of code. Perhaps not really necessary once we actually fix this, or we can implement an audit that we expose to project members somehow, that could also audit the app as well as the server.
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.
The
attachment.nonokevent only fires when someone asks for an attachment, so a photo that never finished syncing stays invisible until a partner's connector trips over it. #79 ran the audit endpoint on an hourly timer and sent every result to Sentry; the review pointed out that a server can sit without a sync for days, so that is mostly noise and wasted reads.This draft runs the audit after a sync instead.
src/attachment-audit-scheduler.jslistens to each project'ssync-stateevent and treats a sync session as ended when a device that had data sync enabled drops out ofremoteDeviceSyncState(its connection closed) or when the project goes from wanting data blocks to wanting none. It waits five seconds in case another device is still mid-sync, runs the audit at most once perAUDIT_MIN_INTERVAL_MSper project (default one hour,0disables), and when a sync ends sooner than that it schedules the audit for the earliest allowed time rather than dropping it. Nothing runs at start. The counts are logged at info level on every run; Sentry gets oneattachment.auditevent per run, with the counts in the event context and the failures and device names as extra data, only when something is missing. Projects already on the server are watched after listen, and projects added byPUT /projectsare handed to the scheduler through a newonProjectAddedserver option. The scheduler takes an injected audit function and is unit tested with node's mock timers, so the test needs no blobs.Scope and limits. A device that only did presync, including the brief first connection while it is being added to the project, does not count as a sync ending; it changed no data. The scheduler runs only when
SENTRY_DSNis set, as the hourly audit did, since logging counts on a server nobody is watching is the same wasted read. The one-hour floor counts from when the last audit started, so a server with constant sync traffic audits at most hourly. The server'ssync-stateevent is throttled by core, so "wanting nothing" can arrive a few hundred milliseconds after the last block.Related changes. This replaces the periodic audit part of #79 and is the change the review asked for in its place. It stacks on the Sentry reporting PR (#89) and through it on the audit endpoint PR (#88). Once core exposes a blob status API the audit can stop downloading bytes and the cap can go.