← Back to PRs

#16573: fix(signal): increase attachment fetch timeout to 30s (#16545)

by robbyczgw-cla open 2026-02-14 21:49 View on GitHub →
channel: signal size: XS experienced-contributor
## Summary Fixes #16545 — `fetchAttachment()` in `signal/monitor.ts` calls `signalRpcRequest()` without a `timeoutMs`, falling back to the default 10s. For larger attachments (photos, videos) or slower connections, this causes `AbortError` and the attachment is silently dropped — the agent only sees the text. ## Changes **`src/signal/monitor.ts`** - Add `ATTACHMENT_TIMEOUT_MS = 30_000` constant - Pass `timeoutMs: ATTACHMENT_TIMEOUT_MS` to `signalRpcRequest()` in `fetchAttachment()` ## Why 30s The default RPC timeout (10s) is reasonable for lightweight JSON-RPC calls (send, receive, list), but attachment fetching involves downloading and base64-encoding binary data from Signal's servers. 30s gives enough headroom for ~5MB attachments on slower connections while still failing fast on genuine errors. The `signalRpcRequest` function already accepts `timeoutMs` — it just wasn't being passed by `fetchAttachment()`. --- ## Local Validation - `pnpm build` ✅ - `pnpm check` (oxlint) ✅ - Relevant test suites pass ✅ ## Contribution Checklist - [x] Focused scope — single fix per PR - [x] Clear "what" + "why" in description - [x] AI-assisted (Codex/Claude) — reviewed and tested by human - [x] Local validation run (`pnpm build && pnpm check`) *AI-assisted (Claude). Reviewed by human.* <!-- greptile_comment --> <h3>Greptile Summary</h3> Increases the timeout for `fetchAttachment()` in the Signal monitor from the default 10s RPC timeout to 30s. The `signalRpcRequest` function already supported a `timeoutMs` parameter — it just wasn't being passed by `fetchAttachment()`, causing larger attachments (photos, videos) to silently fail with `AbortError` on slower connections. - Adds `ATTACHMENT_TIMEOUT_MS = 30_000` constant in `src/signal/monitor.ts` - Passes `timeoutMs: ATTACHMENT_TIMEOUT_MS` to `signalRpcRequest()` in `fetchAttachment()` - No issues found — clean, minimal, well-scoped fix <h3>Confidence Score: 5/5</h3> - This PR is safe to merge — it's a minimal, single-line behavior change that uses an already-supported parameter. - The change is extremely focused: it adds one constant and passes it to an existing parameter. The `signalRpcRequest` function already supports `timeoutMs`, the caller (`fetchAttachment`) already has proper error handling via try/catch in the event handler, and existing tests mock `fetchAttachment` so they are unaffected. No new code paths, no API changes, no risk of regression. - No files require special attention. <sub>Last reviewed commit: 0e1a924</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs