feat(desktop): say "attachment expired" instead of showing a broken file - #31
Merged
Conversation
An old message can outlive its own attachment. Until now that read as a
bug in the app: a broken image frame with no explanation, or a download
button that failed every single time it was pressed, with a toast that
was gone before the next press.
The two paths learn it differently, because they fail differently. An
<img> error event carries no status at all, so the image path asks the
media URL for one byte and classifies the answer. Downloads never touch
the webview — they go through the native command so they traverse the
tunnel — so the file path reads the status out of the rejection string,
which relay_error_message always builds as "relay returned {status}".
404 and 410 are the only two statuses that mean the bytes are not coming
back; everything else is a fault that may clear. Telling someone their
file is gone because the network blinked is the one wrong answer here,
so a 403, a 500 or a thrown fetch stays a plain failure.
…dule markdown.tsx is 1968 lines against a 1000-line ceiling, so the ratchet holds it where it is: the expired-attachment branch cannot land while the file also grows. `getImageLightboxFocusableElements` is pure DOM with no markdown in it and every one of its siblings already lives in ./markdown/imageLightbox — this is where it belonged already.
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.
What
A message can outlive its own attachment. Until now that read as a bug in the app rather than an old message:
Both now render an "Attachment expired" card that keeps the filename — so the message still records what was shared — and offers no action, because there is nothing left to fetch.
How each path learns it
They fail differently, so they ask differently:
ImageBlock<img>errorevent, which carries no statusFileCarddownload_filerejectionThe file path deliberately does not probe.
hrefthere is the unrewritten relay URL —RELAY_MEDIA_REonly matches image/video extensions, so a PDF is never proxied and a webview fetch to it is bounced by Cloudflare. That 403 would classify asunavailableand the card would never learn the truth. The rejection string already carries it:relay_error_messagebuilds every relay error asrelay returned {status}[: message], and that prefix is the stable half.Only 404 and 410 are expiry
Everything else is a fault that may clear. Telling someone their file is gone because the network blinked is the one wrong answer available here, so 403, 429, 5xx and a thrown fetch all stay a plain failure with the normal toast. A message with no status in it at all — a refused connection, a cancelled save dialog, a hash mismatch — returns
nulland changes nothing.Incidentally fixed
FileCardwas doingerr instanceof Error ? err.message : "Download failed". Tauri rejects with the command's rawString, never anError, so every real download error was being discarded and replaced with the generic text.errorText()now handles both shapes.Test plan
desktop/src/shared/lib/mediaAvailability.test.mjs— 7 tests: 404/410 → expired, 200/206/304 → available, 403/429/5xx → unavailable, status parsed out of the relay wording, statusless messages →null, the probe's shape (GET +range: bytes=0-0+no-store), and a throwing fetch → unavailable.npx tsc --noEmit -p tsconfig.jsonclean.npx biome checkclean.