Skip to content

fix: add defensive checks in storage resolvers instead of non-null assertions#965

Open
pyramation wants to merge 1 commit intomainfrom
devin/1775276749-defensive-resolver-check
Open

fix: add defensive checks in storage resolvers instead of non-null assertions#965
pyramation wants to merge 1 commit intomainfrom
devin/1775276749-defensive-resolver-check

Conversation

@pyramation
Copy link
Copy Markdown
Contributor

Summary

Replaces cdn! and credential ! non-null assertions with explicit guard clauses and actionable error messages in both bucket-provisioner-resolver.ts and presigned-url-resolver.ts.

Previously, if CDN env vars were missing, these resolvers would silently pass undefined into the S3 client, producing cryptic errors deep in the AWS SDK. Now they throw early with messages that name the specific missing config.

Both resolvers are lazy (called on first storage operation, not at startup), so servers that don't use storage features are unaffected.

Review & Testing Checklist for Human

  • Verify env var names in error messages match reality — messages reference AWS_ACCESS_KEY, AWS_SECRET_KEY, CDN_BUCKET_NAME. Confirm these match the actual env var names that feed into getEnvOptions().cdn. If the real names differ (e.g. CDN_AWS_ACCESS_KEY), the error messages will be misleading.
  • Confirm lazy throw doesn't crash the server — these resolvers are called from within mutation/resolver handlers. Verify that a thrown error here surfaces as a GraphQL error to the caller rather than crashing the process. The plugin code wraps provisioning calls in try/catch, but the presigned-url plugin path should be checked too.

Notes

  • bucket-provisioner-resolver.ts intentionally does not check for bucketName (it doesn't use one — bucket names are resolved per-row from the database).
  • presigned-url-resolver.ts adds an additional bucketName check since it requires a default bucket for signing URLs.
  • No logic changes — purely guard clauses replacing ! assertions, so existing behavior is preserved when config is present.

Link to Devin session: https://app.devin.ai/sessions/4c882ba2dfbf4045adf85fb83cde6f77
Requested by: @pyramation

…sertions

Replace cdn! and credential! non-null assertions with explicit error
messages in both bucket-provisioner-resolver.ts and presigned-url-resolver.ts.

If CDN env vars are missing, the resolvers now throw clear errors explaining
which variables to set, instead of silently passing undefined to S3 clients.
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

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