Skip to content

Handle an aws.ServeFile error gracefully instead of panicking - #427

Open
bauerpawel wants to merge 2 commits into
Forceu:masterfrom
bauerpawel:fix/aws-servefile-error-handling
Open

bauerpawel wants to merge 2 commits into
Forceu:masterfrom
bauerpawel:fix/aws-servefile-error-handling

Conversation

@bauerpawel

Copy link
Copy Markdown
Contributor

Description

ServeFile called helper.Check(err) on the result of aws.ServeFile, which panics on any error — for example if the object has gone missing from the bucket between the metadata lookup and the actual download, or any other transient S3-side failure. Logs the error and writes a plain error response to the client instead of crashing the request.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Technical Details

  • Database changes: No
  • Storage backend affected: S3
  • Usage of AI: Yes — developed with Claude Code as a pair-programming assistant. I reviewed and tested all changes before submitting.

How Has This Been Tested?

  • Unit Tests: Added TestServeFileAwsErrorHandling, which registers a file with an AWS bucket set that was never actually uploaded (so the mock driver returns "file not found") and asserts ServeFile responds with the error message instead of panicking. Passes in isolation under go test ./internal/storage/... --tags=test,awsmock -run TestServeFileAwsErrorHandling.
  • Environment: Windows 11

Checklist

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation. (n/a — internal error-handling behavior)
  • My changes generate no new warnings.

🤖 Generated with Claude Code

https://claude.ai/code/session_0168YUwGEzEHTqd9CwzXE53d

bauerpawel and others added 2 commits September 9, 2026 14:56
ServeFile previously called helper.Check(err) on the result of
aws.ServeFile, which panics on any error (e.g. the object having gone
missing from the bucket between metadata lookup and download).
Log the error and write a plain error response instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0168YUwGEzEHTqd9CwzXE53d
forceDecryption=false made aws.ServeFile take the redirectToDownload
path, which only presigns a URL without contacting S3, so it never
returns an error for a missing object. Setting forceDecryption=true
routes through serveDecryptedFile, which calls s3.GetObject directly
and surfaces the missing-object error the test is meant to cover.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JWZJU5aSCEBB9DHBWvL4t4
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