Repository navigation
fix(opencode): graceful error handling for PDF/image file read failures - #31329
zhiyiwang-byte wants to merge 3 commits into
Conversation
Previously, when a PDF or image file was unreadable (corrupted, permissions error, truncated, etc.), the session would crash because: 1. prompt.ts used Effect.catch(Effect.die) to convert read errors into unrecoverable defects during user attachment processing 2. read.ts let fs.readFile errors propagate as raw defects through Effect.orDie 3. transform.ts had no validation for empty/corrupted PDF base64 data This commit fixes all three crash paths: - prompt.ts: Replace Effect.catch(Effect.die) with Effect.exit + Exit.isFailure pattern (consistent with text/plain and directory handling in same file), converting read failures into user-visible error messages instead of defects - read.ts: Wrap fs.readFile in Effect.catch to produce a descriptive error message that surfaces through the tool-error event to the model - transform.ts: Add empty base64 PDF data check (mirrors existing empty image check) to catch corrupted PDFs before they hit the provider API Fixes anomalyco#21390
github-actions
Bot
added
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
@Hona @thdxr @Brendonovich could you please review this PR? It's been open for a week. |
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
Closes #21390
Type of change
What does this PR do?
When a PDF file is unreadable (corrupted, permissions issue, truncated), the session crashes. The root cause is in
prompt.ts:993whereEffect.catch(Effect.die)converts file read IO errors into unrecoverable defects that bypass all error recovery.The same file already handles
text/plainanddirectoryattachments gracefully usingEffect.exit+Exit.isFailure, but the PDF/image path was missing this protection.Changes:
prompt.ts— ReplaceEffect.catch(Effect.die)withEffect.exit+Exit.isFailure(matches the existing pattern for text/directory in the same function). On failure, logs the error, publishes an error event, and returns a text message instead of crashing.read.ts— Wrapfs.readFileinEffect.catchso when the Read tool fails to read a PDF, the error message is descriptive (e.g. "Cannot read PDF file: /path (permission denied)") rather than an opaque defect.transform.ts— Add empty base64 PDF validation (mirrors the existing empty image check at lines 381-393) to catch corrupted PDFs before they reach the provider API.How did you verify your code works?
tsc --noEmitpasses with no errors on all modified filesprompt.tsdefect → session crash is eliminated by usingEffect.exitwhich is the same pattern already proven working for text/directory in the same functioncheck-standardsandcheck-complianceCI checks passScreenshots / recordings
N/A — not a UI change.
Checklist