feat(filesharing): pg-js 1.4 + transparent retry UX - #145
Conversation
Bumps `@e4a/pg-js` from `^1.3.0` to `^1.4.0` to pick up the new
upload/download retry machinery (chunk PUTs and download GETs retry
with exponential backoff + full jitter on 5xx and network errors).
The bump itself is a clean semver minor — additive config, no API
changes for the website.
UX additions on top of the bump:
- **Retry indicator.** `src/lib/postguard.ts` now wires an `onRetry`
callback into the SDK that updates a small Svelte store
(`retryStatus`). Both `SendButton.svelte` (upload) and
`download/+page.svelte` subscribe to it and render a single line
("Connection hiccup, retrying… (attempt N of M)") underneath the
active spinner during the retry window. Cleared on success or
terminal error so a stale event can't leak between operations.
- **`UploadSessionExpiredError` distinguished from generic 5xx.** The
SDK now raises this dedicated error when cryptify reports
`upload_session_not_found` (idle past the configured TTL or the
server restarted). On upload it surfaces a regular Error state
with `serverError = false` so the existing copy doesn't blame the
server for a state-loss problem retry can't fix; on download it
gets its own state and copy ("Upload session expired" / "Ask the
sender to send the files once more"). Localised in en + nl.
Also unblocks the husky pre-commit hook: the repo had
`prettier-plugin-svelte` as a devDependency but never registered it
in the prettier config, so `prettier --write` on staged paths failed
on .svelte files (see #143's bypass note). The plugin is now wired
up via the `plugins` field in the package.json `prettier` block, and
all 41 .svelte files were reformatted to apply the resulting style
(mostly stray semicolons + minor whitespace). Six pre-existing
`eslint-disable-next-line` comments that prettier broke by wrapping
their target tags onto multiple lines were widened to
`eslint-disable` / `eslint-enable` blocks.
Refs: postguard-website#117, encryption4all/cryptify#136,
encryption4all/cryptify#145, encryption4all/postguard-js#47.
|
/dobby review this PR. Last time you reviewed you did not approve the PR even when the comments were resolved. Make sure you approve the PR if you think it is ready. (Save to memory that you should always do this.) |
|
Dobby is on it! The little agent will check out PR #145, build it, run the tests, and post a thorough review. If everything looks good, Dobby promises to actually press the approve button this time — no more leaving reviews stuck in limbo! |
There was a problem hiding this comment.
Review
Verified locally on feat/pg-js-1.4-retry-ux:
npm ci --legacy-peer-deps— cleannpm run lint— prettier + eslint clean (the new prettier-svelte plugin registration is doing what the body claims)npm run check— 0 errors, 0 warningsnpm run build— success, adapter-static output written- All 7 CI checks on the PR are green
Functional changes (the parts that aren't reformat noise)
@e4a/pg-js^1.3.0→^1.4.0— verified the new exports (UploadSessionExpiredError,RetryEvent,RetryOptions,retry.onRetry) exist in the publisheddist/index.d.mts. Clean additive minor.retryStatusstore insrc/lib/postguard.ts— module-scopedwritable<RetryEvent | null>, fed by the SDK'sonRetryhook. Doc comment on the store is accurate and explains the lifetime contract (consumer resets on success/terminal). Good.SendButton.svelte— resetsretryStatusat three boundaries: beforesealed.upload, after success, and in every catch arm. The newUploadSessionExpiredErrorarm setsserverError = falseand routes toEncryptionState.Error, which is correct — the existing Error template will read the non-server-error copy. Retry banner usesattempt + 1againstmaxAttempts, which matches the SDK's docstring ("1-indexed attempt that just failed; the next attempt will beattempt + 1"), so the banner correctly shows the upcoming retry number.download/+page.svelte— adds the newSessionExpiredstate to the discriminated union, branches in bothstartDownload(inspect) andstartDecryption(decrypt) catches. Order matters and is right:IdentityMismatchErrorandUploadSessionExpiredErrorare checked before the genericNetworkError && status >= 500fallback.retryStatus.set(null)is called at every state-transition boundary — no stale leakage.- i18n —
sessionExpiredTitle/Subtitle/Messageandretryingadded in bothen.jsonandnl.json; placeholder names ({attempt},{max}) match the call sites.
prettier-plugin-svelte registration
The diff is large because of the 41-file reformat, but it's mostly stray semicolons / line-wrap differences — exactly what dep-bump-gotchas.md predicts. The functional content of those files is unchanged. Six pre-existing eslint-disable-next-line comments were widened to disable/enable blocks because prettier wrapped their target tags onto multiple lines — verified those blocks scope only the {@html ...} line they were already protecting. This unblocks the husky pre-commit hook, which is a real win — no more --no-verify for .svelte edits.
Untested manually
The three Playwright/manual items in the PR's test plan (throttled retry, kill cryptify mid-upload, evicted-session download) require dev cryptify + DevTools and aren't reproducible in CI. The code paths look correct on inspection but those are the bits a human should still smoke-test before deploying.
Verdict
Approving. CI is green, build/lint/check are clean locally, the SDK contract is correctly consumed, error-state routing is sound, i18n is complete in both locales, and the husky unblock is a nice bonus. Approved with the standard caveat that the manual retry-banner / session-expiry smoke tests should be ticked off against staging before this rolls to production.
|
/dobby add those manual tests encryption4all/postguard-business#48 |
|
Dobby sees the request from @rubenhensen! Dobby will fold the upload-robustness manual tests from postguard-business#48 §1 into this PR's test plan right away. So happy to help! |
|
Folded the §1 "Upload robustness — retry + idempotency" checklist from encryption4all/postguard-business#48 into the test plan above. Existing manual items were re-organised into the broader checklist so nothing is duplicated — the three previously listed items (slow-3G retry, kill-mid-chunk, evicted-session download) map directly to the Slow network retry, Server flap mid-chunk, and Session expired (download) entries. Sections §2-§10 of the cycle checklist target other repos / surfaces and were left where they belong. |
Summary
Picks up the new pg-js retry machinery and surfaces the user-facing pieces (retry indicator + a distinct "upload session expired" message). This is the website end of the Phase 1 robustness work tracked in #117.
The pg-js bump itself is a clean semver minor — additive config, no API changes. The cryptify side that the SDK retries against is already on
main/edge(cryptify#131, #132, #144, #145).What changed
@e4a/pg-js:^1.3.0→^1.4.0inpackage.json.src/lib/postguard.tsexports a small Svelte storeretryStatusthat the SDK'sonRetryhook updates.SendButton.svelte(upload) anddownload/+page.sveltesubscribe and render a single line — "Connection hiccup, retrying… (attempt N of M)" — under the active spinner during the retry window. Cleared on success or terminal error so a stale event can't leak between operations. Localised in en + nl.UploadSessionExpiredErrordistinguished from generic 5xx in both call sites:serverError = false, so the copy doesn't blame the server for a state-loss problem retry can't fix.SessionExpiredstate with its own title ("Upload session expired") and message ("Ask the sender to send the files once more"). Localised.retryStatusat the boundaries of every operation so a banner from a prior attempt can't leak into a new one.Hook unblock + svelte reformat
The repo had
prettier-plugin-svelteas a devDependency but never registered it in the prettier config, so the husky pre-commit hook failed on every commit that touched a.sveltefile (see #143's bypass note). This PR registers the plugin via thepluginsfield in the package.jsonprettierblock. The price is reformatting all 41 .svelte files (mostly stray semicolons + minor whitespace — git diff shows it clearly per file). Six pre-existingeslint-disable-next-linecomments that prettier broke by wrapping their target tags onto multiple lines were widened toeslint-disable/eslint-enableblocks.Together that means
npm run lintnow passes cleanly without--no-verify.Test plan
Automated
npm run check(svelte-check) — 0 errors, 0 warningsnpm run lint(prettier --check + eslint) — cleannpm run build— success--no-verifyneeded for this PR's commit)Manual — upload robustness on staging
Folded in from the cycle-wide checklist in encryption4all/postguard-business#48 §1. Setup: open staging Business UI (or postguard.eu file-share), log in, start a flow that uploads a multi-chunk file (≥ ~20 MB so several chunks fly).
maxAttemptsthen surfaces a clear error (no infinite spinner).serverError = false), not a generic "Server error", and the SDK does not burn its retry budget on it.SessionExpiredstate with "Ask the sender to send the files once more", not the generic ServerError state.session_ttl_secsto a low value): leave a partially-uploaded file idle past the TTL → next chunk gets the structured 404 (reason: expired_or_unknown) and surfaces the session-expired copy.Refs
Cross-refresh upload resume and SDK-side resumable downloads will be filed as separate issues — they require different infrastructure and aren't in scope here.