Fix grapher 404 page #3970
Fix grapher 404 page #3970
Conversation
|
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 0 ✅ Edited: 2024-09-17 15:06:22 UTC |
This is an alternative fix for the one reverted in 24f6288.
rakyi
force-pushed
the
fix-grapher-404
branch
from
35c6bb9
to
53224b7
Compare
| @@ -146,7 +102,7 @@ async function handleHtmlPageRequest( | |||
| const grapherPageResp = await env.ASSETS.fetch(url, { redirect: "manual" }) | |||
|
|
|||
| if (grapherPageResp.status === 404) { | |||
| throw new StatusError(404) | |||
| return handlePageNotFound(env, grapherPageResp) | |||
There was a problem hiding this comment.
One thing I noticed while tinkering with the code, the For local testing commented-out code block above here could be updated to call fetch from env.ASSETS
There was a problem hiding this comment.
I think a comment here would be useful that mentions that by passing in the grapherPageResp as the fallback we get a 404 status code but also an HTML 404 page. Otherwise this subtle detail might be lost in bigger refactorings in the future.
There was a problem hiding this comment.
Added a comment about grapherPageResp.
commented-out code block above here could be updated to call
fetchfromenv.ASSETS
Even if it works, I think it makes sense to leave it to a normal fetch, since we are not fetching from the assets (semantically), we are fetching from the internet.
| @@ -146,7 +102,7 @@ async function handleHtmlPageRequest( | |||
| const grapherPageResp = await env.ASSETS.fetch(url, { redirect: "manual" }) | |||
|
|
|||
| if (grapherPageResp.status === 404) { | |||
| throw new StatusError(404) | |||
| return handlePageNotFound(env, grapherPageResp) | |||
There was a problem hiding this comment.
I think a comment here would be useful that mentions that by passing in the grapherPageResp as the fallback we get a 404 status code but also an HTML 404 page. Otherwise this subtle detail might be lost in bigger refactorings in the future.
This is an alternative fix for the one reverted in 24f6288.