Routing error on canary when using asset prefix #15188
Assignees
Milestone
Comments
7 tasks
kodiakhq bot
pushed a commit
that referenced
this issue
Jul 19, 2020
Fixes #15188 `parseRelativeUrl` was used on urls that weren't always relative. It was used to generate a cache key, but we actually don't need these cache keys to be relative if the urls aren't relative. Also took a look at the overall static data fetching logic and found a few things: - [x] cache key is unnecessarily transformed through `prepareRoute`, we can just cache by resolved `dataHref` and remove that function. Pretty sure that `prepareRoute` was also introducing edge cases with `assetPath` and `delBasePath` - [x] there is [a bug in the caching logic](https://github.com/vercel/next.js/blob/ebdfa2e7a3f8e22e03b94dfb5f00481bf06254b6/packages/next/next-server/lib/router/router.ts#L898) that made it fail on the second visit: it should be `Promise.resolve(this.sdc[pathname])` instead of `Promise.resolve(this.sdc[dataHref])`. Also added a test for this - [x] ~converted to async await to improve stacktraces and readability.~ I assumed this was fine since I saw some async/awaits in that file already but it seems to just blow up the size of the non-modern bundle. - [x] extracted nested `getResponse` function and define it top level. this should improve runtime performance - [x] convert `_getStaticData` and `_getServerData` to class methods instead of properties. Not sure why they were defined as properties but I think they belong on the prototype instead. - [x] remove `cb` property from `fetchNextData`, it's unnecessary and makes the async flow hard to understand. The exact same logic can go in the `.then` instead. - [ ] data fetching logic [retries on 5xx errors](https://github.com/vercel/next.js/blob/ebdfa2e7a3f8e22e03b94dfb5f00481bf06254b6/packages/next/next-server/lib/router/router.ts#L157), but not on network level errors. It should also retry on those. It should also not retry on every 5xx, probably only makes sense on 502, 503 and 504. (e.g. 500 is a server error that I wouldn't expect to succeed on a retry) The overall result also is a few bytes smaller in size
|
This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Bug report
Describe the bug
See #14827 (comment)
Happens when an absolute
assetPathis configured and on links to pages that havegetStaticPropsThe text was updated successfully, but these errors were encountered: