Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves error handling in the service worker by properly returning a network error response instead of throwing an uncaught error when navigation requests fail for non-PWA users or when PWA users don't have a cached offline page.
Changes:
- Replaced
throw errwithreturn Response.error()in the navigation fetch error handler to properly handle network failures
| } | ||
| // fallback to browser default behavior | ||
| throw err; | ||
| return Response.error(); |
There was a problem hiding this comment.
The change from throwing an error to returning Response.error() is correct for service worker error handling. However, test coverage is missing for this specific error scenario. The existing test in src/sw.test.js only covers the case where navigation fails for PWA users who have an offline page cached. There should be a test case for when navigation fails for non-PWA users or PWA users without a cached offline page to verify that Response.error() is returned correctly.
| @@ -170,8 +170,7 @@ const fetchEventHandler = async event => { | |||
| return cachedOffline; | |||
| } | |||
| } | |||
There was a problem hiding this comment.
The comment "fallback to browser default behavior" has been removed. While the new approach using Response.error() is correct, consider whether a comment explaining the error handling strategy would be helpful. For example: "Return network error response for failed navigation requests" to clarify that this intentionally returns an error response rather than attempting to show a generic offline page.
| } | |
| } | |
| // Return network error response for failed navigation when no offline page is available |
1 participant
Resolves JIRA:
Summary
adding error handler for uncached promise
Code changes
Testing
Useful Links