@remotion/licensing: No unhandled rejection if usage event send fails#6203
@remotion/licensing: No unhandled rejection if usage event send fails#6203JonnyBurger merged 4 commits intomainfrom
@remotion/licensing: No unhandled rejection if usage event send fails#6203Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent unhandled promise rejections when usage event tracking fails due to network errors by adding network error detection and logging instead of throwing errors.
- Adds a new
isNetworkErrorutility function to detect network failures based on error messages - Updates
registerUsageEventto catch and log network errors - Adds
licenseKey: 'free-license'to a test case to ensure usage events are sent during testing
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/licensing/src/is-network-error.ts |
New utility function that detects network errors by checking error messages from different browsers |
packages/licensing/src/register-usage-event.ts |
Imports and uses isNetworkError to log network failures with console.log |
packages/web-renderer/src/test/frame-range.test.tsx |
Adds licenseKey parameter to test to enable usage event tracking in test scenarios |
Comments suppressed due to low confidence (1)
packages/licensing/src/register-usage-event.ts:101
- Network errors are logged but still re-thrown at line 101, which defeats the purpose of preventing unhandled rejections. After logging a network error, the function should return a default response instead of throwing. Consider returning a value like
{ billable: false, classification: 'failed' }for network errors to allow the caller to continue without handling the error.
if (isNetworkError(err as Error)) {
// eslint-disable-next-line no-console
console.log('Failed to send usage event', err);
}
clearTimeout(timeout);
if (err instanceof Error && err.name === 'AbortError') {
throw new Error('Request timed out after 10 seconds');
}
| const read = await res.json(); | ||
| return read; |
There was a problem hiding this comment.
The response body can only be read once. Line 76 already calls res.json() to parse the response. Lines 89-90 attempt to call res.json() again, which will fail because the body stream has already been consumed. This code appears to be unreachable since all success paths return at line 79-82, and all error paths throw at line 86. Consider removing lines 89-90.
|
@claude fix this: The response body can only be read once. Line 76 already calls res.json() to parse the response. Lines 89-90 attempt to call res.json() again, which will fail because the body stream has already been consumed. This code appears to be unreachable since all success paths return at line 79-82, and all error paths throw at line 86. Consider removing lines 89-90. |
|
Claude finished @JonnyBurger's task in 58s —— View job Fixed unreachable code in register-usage-event.ts ✓Tasks
SummaryRemoved lines 89-90 from
The fix has been committed and pushed to the --- | |
Co-authored-by: Jonny Burger <JonnyBurger@users.noreply.github.com>
|
@claude now we return nothing. throw an error instead |
|
Claude finished @JonnyBurger's task in 1m 1s —— View job Fixed error handling in register-usage-event.ts ✓Tasks
SummaryAdded The fix has been committed and pushed to the |
Co-authored-by: Jonny Burger <JonnyBurger@users.noreply.github.com>
Only a log message