Properly handle in-flight network errors on _data requests #6783
Conversation
🦋 Changeset detectedLatest commit: 4321b36 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
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 |
| init.headers = headers; | ||
| return new Response(body, init); | ||
| } | ||
|
|
||
| // Mark all successful responses with a header so we can identify in-flight | ||
| // network errors that are missing this header | ||
| response.headers.set("X-Remix-Response", "yes"); |
There was a problem hiding this comment.
Instead of having to remember to do this in each success code path,. should we instead do this higher up? I kept it here to colocate it with the setting of the error/catch headers but it feels potentially error-prone in the future:
function requestHandler() {
...
if (url.searchParams.has("_data")) {
response = await handleDataRequestRR(...);
if (!response.headers.has('X-Remix-Error') && !response.headers.has('X-Remix-Catch')) {
response.headers.set('X-Remix-Response', 'yes');
}
...
}
...
}
|
@jacob-ebey raised a good question around whether a CDN would cache this header or not. I think it probably depends on the CDN - I would hope CDN's aren't caching 4xx/5xx responses. But technically, yeah if a if a CDN cached a user-returned 4xx/5xx response and did not cache the |
|
Spoke with @mjackson and we agreed that CDN's (1) should not generally be caching 4xx/5xx responses, but (2) if they are configured to - they should be caching the headers along with it, so we do not expect the above scenario to be an issue. |
github-actions
bot
added
the
awaiting release
|
🤖 Hello there, We just published version Thanks! |
|
🤖 Hello there, We just published version Thanks! |
|
🤖 Hello there, We just published version Thanks! |
github-actions
bot
removed
the
awaiting release
If we make a
?_datafetchand it never actually reaches the Remix server, and instead we get a 4xx/5xx response back from a CDN/Proxy/Load Balancer, etc., then we currently incorrectly think it's a returned response from aloader/actionsince it's perfectly valid to:This PR adds an
X-Remix-Response: yesheader to successful responses for?_datarequests so we can differentiate responses that reached the Remix server from those that did not - and we can treat CDN/Proxy errors as real errors and surface them to theErrorBoundary.Closes #5418