-
Notifications
You must be signed in to change notification settings - Fork 329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: improve error handling in utapi #1125
Conversation
🦋 Changeset detectedLatest commit: 3fe8445 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe pull request focuses on enhancing error handling within the uploadthing component, specifically in the utapi module. The changes introduce improved error logging and management across multiple files, including the upload server, SDK utilities, and test helpers. The modifications ensure that errors are properly captured, logged, and reported during file upload processes, providing better visibility into error occurrences. Changes
Sequence DiagramsequenceDiagram
participant Client
participant UploadServer
participant ErrorLogger
participant HTTPClient
Client->>UploadServer: Initiate File Upload
UploadServer->>HTTPClient: Send Upload Request
alt Upload Successful
HTTPClient-->>UploadServer: Upload Complete
UploadServer-->>Client: Success Response
else Upload Fails
HTTPClient-->>ErrorLogger: Log HTTP Client Error
ErrorLogger->>UploadServer: Error Details
UploadServer-->>Client: Structured Error Response
end
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
More templates
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/uploadthing/src/_internal/upload-server.ts (1)
9-9
: LGTM! Consider enhancing the error message.The addition of error logging before error transformation is a good practice. However, the error message could be more descriptive.
Consider including more context in the error message:
- Effect.tapError(logHttpClientError("Failed to upload file")), + Effect.tapError(logHttpClientError(`Failed to upload file: ${file.name}`)),Also applies to: 28-28
packages/uploadthing/test/sdk.test.ts (1)
102-140
: LGTM! Consider adding more error scenarios.The test case effectively verifies the error handling for failed uploads. The implementation is clean and thorough.
Consider adding test cases for:
- Network errors (timeout, connection refused)
- Different HTTP error codes (500, 503)
- Malformed server responses
Example test case:
it("handles network errors gracefully", async () => { const mockedIngestUrl = "https://mocked.ingest.uploadthing.com"; msw.use( http.all<{ key: string }>( `${mockedIngestUrl}/:key`, async () => { return HttpResponse.error(); }, ), ); const utapi = new UTApi({ token: testToken.encoded, ingestUrl: mockedIngestUrl, }); const result = await utapi.uploadFiles(fooFile); expect(result.error?.code).toBe("UPLOAD_FAILED"); msw.use(...handlers); });.changeset/curvy-rings-own.md (1)
5-7
: Enhance the changeset description with more details.While the description captures the essence of the change, it would be more helpful for future reference if you could expand it to include:
- Specific examples of errors that were being ignored
- Description of the new error handling behavior
- Impact on existing implementations
Here's a suggested enhancement:
-fix: improve error handling in utapi - -sometimes errors got silently ignored when they shouldn't have +fix: improve error handling in utapi + +Previously, certain errors during file upload processes were being silently ignored, +particularly during HTTP client errors and presigned URL generation. This patch: + +- Adds proper error logging for HTTP client errors +- Improves handling of UploadThingError and ConfigError +- Ensures errors are properly propagated instead of being silently ignored + +This change improves debugging capabilities but doesn't introduce breaking changes +to existing implementations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/curvy-rings-own.md
(1 hunks)packages/uploadthing/src/_internal/upload-server.ts
(3 hunks)packages/uploadthing/src/sdk/utils.ts
(2 hunks)packages/uploadthing/test/__test-helpers.ts
(1 hunks)packages/uploadthing/test/sdk.test.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: typecheck
- GitHub Check: analyze-bundle (current-pr)
- GitHub Check: e2e-node (backend-adapters)
- GitHub Check: e2e-node (minimal-pagedir)
- GitHub Check: lint
- GitHub Check: e2e-node (minimal-appdir)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (2)
packages/uploadthing/src/sdk/utils.ts (1)
123-125
: LGTM! Robust error handling implementation.The implementation properly handles specific error types and converts them to a consistent format. The error messages are clear and the error handling is comprehensive.
Also applies to: 134-136
packages/uploadthing/test/__test-helpers.ts (1)
67-67
: LGTM! Good test organization.Exporting the
callRequestSpy
function improves code reusability across test files.
📦 Bundle size comparison
|
Summary by CodeRabbit
Bug Fixes
Tests
Chores