Skip to content

feat(attachments): audit attachments after each sync and report missing ones to Sentry - #90

Draft
gmaclennan wants to merge 2 commits into
feat/sentry-attachment-reportingfrom
feat/sentry-attachment-audit-after-sync
Draft

gmaclennan wants to merge 2 commits into
feat/sentry-attachment-reportingfrom
feat/sentry-attachment-audit-after-sync

Conversation

@gmaclennan

@gmaclennan gmaclennan commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

The attachment.nonok event 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.js listens to each project's sync-state event and treats a sync session as ended when a device that had data sync enabled drops out of remoteDeviceSyncState (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 per AUDIT_MIN_INTERVAL_MS per project (default one hour, 0 disables), 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 one attachment.audit event 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 by PUT /projects are handed to the scheduler through a new onProjectAdded server 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_DSN is 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's sync-state event 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.

@gmaclennan gmaclennan left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

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.

1 participant