Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow manually redirecting to 404 route within a dynamic slug #13788

Closed
misaon opened this issue Apr 22, 2022 · 20 comments
Closed

allow manually redirecting to 404 route within a dynamic slug #13788

misaon opened this issue Apr 22, 2022 · 20 comments

Comments

@misaon
Copy link
Contributor

misaon commented Apr 22, 2022

Environment

------------------------------
- Operating System: `Linux`
- Node Version:     `v16.14.2`
- Nuxt Version:     `3.0.0-rc.1`
- Package Manager:  `npm@7.17.0`
- Builder:          `vite`
- User Config:      `-`
- Runtime Modules:  `-`
- Build Modules:    `-`
------------------------------

Reproduction

https://stackblitz.com/edit/github-yyumyt?file=pages/404.vue

Describe the bug

I have defined three pages (routes):

  • car.vue
  • [slug].vue
  • 404.vue

If I go to http://.../hi => it loads [slug].vue => OK
If I go to http://.../car => it loads car.vue => OK
If I go to http://.../404 => it loads [slug].vue => FAIL

Additional context

No response

Logs

No response

@danielroe
Copy link
Member

danielroe commented Apr 22, 2022

~/pages/404.vue is a special route that handles otherwise unmatched routes: see https://v3.nuxtjs.org/guide/directory-structure/pages#catch-all-route.

@misaon
Copy link
Contributor Author

misaon commented Apr 22, 2022

If I have logic in setup inside [slug].vue where I call useFetch and the api returns 404, I would like to redirect to the /404 route which is not possible now.

@misaon
Copy link
Contributor Author

misaon commented Apr 22, 2022

@danielroe Example: I have a bike shop. A customer accesses the route: http://bike.com => index.vue. The customer wants to view the bike categories http://bike.com/blue-bikes => [slug].vue. The customer selects one of the bikes from the category, http://bike.com/best-blue-bike.html => [slug].html.vue. Everything works great, but how do I catch the condition where I want to redirect to a 404 page?

@danielroe
Copy link
Member

which is not possible now.

Are you saying this used to work? Would you provide an example of what you're wanting to do?

(I'm asking as this is a key behaviour and we need to make this as straightforward as we can, so we either need to improve DX or document some examples.)

@danielroe danielroe changed the title Route /404 fails when catch all [...slug].vue is defined allow manually redirecting to 404 route within a dynamic slug Apr 22, 2022
@Aareksio
Copy link
Contributor

Aareksio commented Apr 22, 2022

I am positive the broad idea of this issue is to ask "how to handle 404 on dynamic path".

  const $route = useRoute()
  const { data } = await useAsyncData(`product:${$route.params.id}`, () => $fetch(`/api/products/${$route.params.id}`))

  // TODO: Display proper 404 when accessing unknown product id

  // Requirements:
  // - Using default error page - big "404" with small "Page not found: /product/42" - just like native nuxt 404
  // - On all types of navigation:
  //   a) Home -> Product/42 (client)
  //   b) Home -> Product/42 -> Home -> Product/42 (client + cache)
  //   c) Product/42 (server)
  //   d) Product/42 -> Home -> Product/42 (server into client + cache)
  // - Proper status code and status message when SSR (404, Not Found)

There is currently no documentation on how to properly handle API's 404 response in the docs. From my experience it is also extremely hard, as all of the 4 navigation types listed in comments above require different logic each.


Here is the repo reproduction in case you want to bootstrap the project: data-fetching

@danielroe
Copy link
Member

Thanks for that link @Aareksio.

I've submitted a related PR to fix an issue, add some basic docs and improve DX (by avoiding import of createError), but check out https://stackblitz.com/edit/github-jwfbey. Does that answer your questions?

@Aareksio
Copy link
Contributor

You got around checking for 404 in pretty clever way @danielroe!

This surely answers on how to display 404 when the server returns an error, but we may need different handling for different kind of errors, eg. 5xx, 403, 429 all should stay on the path and return specific status codes. In all cases, the data will be empty and with demonstrated approach we'll end up sending 404. This may signal crawler bots to delist URL despite the real cause being server malfunction or rate limit. Here's rebuilt example covering error code: https://stackblitz.com/edit/github-jwfbey-kyfof4

I think your solution answers this particular issue, as the OP didn't seem to be concerned with proper status code.

@Allypost
Copy link

@danielroe Even with your fix, when using SSR the backend still throws internal an error (and flashes it until the hidration takes over):

[nitro] [dev] [unhandledRejection] Error: nuxt instance unavailable
    at Module.useNuxtApp (file:///home/projects/nuxt-starter-nlacpe/.nuxt/dist/server/server.mjs:416:13)
    at Module.clearError (file:///home/projects/nuxt-starter-nlacpe/.nuxt/dist/server/server.mjs:1004:41)
    at eval (file:///home/projects/nuxt-starter-nlacpe/.nuxt/dist/server/server.mjs:3106:33)
    at triggerAfterEach (/home/projects/nuxt-starter-nlacpe/node_modules/vue-router/dist/vue-router.cjs.js:3156:13)
    at eval (/home/projects/nuxt-starter-nlacpe/node_modules/vue-router/dist/vue-router.cjs.js:3059:13)
    at async eval (file:///home/projects/nuxt-starter-nlacpe/.nuxt/dist/server/server.mjs:2584:7)
    at async createNuxtAppServer (file:///home/projects/nuxt-starter-nlacpe/.nuxt/dist/server/server.mjs:60:7)
    at async Object.renderToString (file:///home/projects/nuxt-starter-nlacpe/node_modules/vue-bundle-renderer/dist/index.mjs:277:19)
    at async eval (file:///home/projects/nuxt-starter-nlacpe/.nuxt/dev/index.mjs:476:20)
    at async eval (file:///home/projects/nuxt-starter-nlacpe/node_modules/h3/dist/index.mjs:475:19)

It's visible in the logs of the example you provided, but here's a minimal example nontheless: https://stackblitz.com/edit/nuxt-starter-nlacpe

nuxt-3-rc3-throwError-ssr-issue.mp4

The error is thrown whenever throwError is used after an await in the setup function

@AndersNielsen85
Copy link

@danielroe the throwError seems to be marked as deprecated now.

Swapping for throw createError seems to output fails in the server console "[nitro] [dev] [unhandledRejection] Error: nuxt instance unavailable

if (!data.value) {
  throw createError({ statusCode: 404, statusMessage: 'Page Not Found' })
}

@danielroe
Copy link
Member

Seems working fine on latest RC8 to me, without any workarounds: https://stackblitz.com/edit/nuxt-starter-fbuud2. Let me know if not and I'll reopen.

@AndersNielsen85
Copy link

@danielroe indeed it works on the initial "faulty" load.

It seems to me it is failing if I am doing some clientside navigation to something faulty though.
So navigating from index to test404
https://stackblitz.com/edit/nuxt-starter-mdhxps?file=pages/index.vue

Is just a limitation to be aware of, or should it be working?

@danielroe
Copy link
Member

danielroe commented Aug 16, 2022

We deliberately don't throw a full page error in this case by default (so you can handle it within the UI with more granular error handlers). But if you want the full error page, you can add fatal: true to the options of createError.

@some-user123
Copy link

I'm having a related problem rendering a 404 error:

<!-- test404.vue -->
<template>
  <div>Content: {{ data.id }}</div>
</template>

<script lang="ts">
export default defineComponent({
  async setup() {
    const { data, error } = await useAsyncData<{ id: number }>(async () => {
      // URL returns 404, causing fetch to throw an error
      return await $fetch('https://run.mocky.io/v3/b764d683-f7cd-4128-9de0-077d891d3886')
    })
    if (error.value) {
      console.log('error detected')
      throw createError({
        statusCode: 404,
        fatal: true,
      })
    }

    return { data }
  },
})
</script>

The console shows:

error detected
[Vue warn]: Unhandled error during execution of setup function
  at <Test404 onVnodeUnmounted=fn<onVnodeUnmounted> ref=Ref< undefined > >
[Vue warn]: Property "data" was accessed during render but is not defined on instance.
[nuxt] [request error] [unhandled] [500] Cannot read properties of undefined (reading 'id')

So, instead of 404, a 500 error is rendered, because the template is trying to access a property of undefined (data.id). It works (404 rendered) if I remove data.id from the template.
Is this the intended behavior?

I'd like to avoid to wrap my whole template into a <div v-if="data"></div> as the template should not be rendered at all (I'm throwing an exception even before the return) and I'm using useAsyncData instead of useLazyAsyncData to not have to handle the empty data situation in the template.

@Aareksio
Copy link
Contributor

Minimal reproduction for the above, using script setup for readability: https://stackblitz.com/edit/github-5jhffm

@Aareksio
Copy link
Contributor

@danielroe I am afraid the error handling of API responses is still not perfect. This time no shortcuts, need to handle both 404 and 500 properly (read error value): https://stackblitz.com/edit/github-cmeky2?file=pages/movie/[id].vue

@danielroe
Copy link
Member

@Aareksio This seems to work for me. As far as retrying requests in certain conditions, you can provide an onResponseError callback in $fetch options for more fine grained handling of issues.

const { data: movie, error } = await useAsyncData(`movie:${$route.params.id}`, () => $fetch(`/api/movie/${$route.params.id}`));

if (error.value && error.value.response) {
  throw createError({
    fatal: true,
    statusCode: error.value.response.status
  })
}

Please feel free to open a feature request if you think something is missing. Though do note that this overlaps slightly with #12885, which I think is a valid issue we need to improve.

@Aareksio
Copy link
Contributor

Aareksio commented Aug 24, 2022

@danielroe https://yko.im/1_it.mp4
Shall I open issue for this, or #12885 already covers this?

Case b: Home -> Movie/[id] -> Home -> Movie/[id] (client + cache)
And also new case: Movie/[id] -> Home -> Movie/[id] -> Home -> Movie/[id] (ssr + client + cache)

Breaks after repetitive visit after client request.


By "retry request" I did not meant retry directly after fail. On repetitive view data is loaded from cache, meaning the request never hits the server.

Imagine the user gets 419 (rate limit) when browsing user profiles, we display proper error message. They proceed to browse other, not limited, parts of the app. After 30 minutes they decide to check the profile again. On server the rate limit is long gone, but client do not even hit the API and instead display cached error (assuming the issue from video gets fixed).

@danielroe
Copy link
Member

danielroe commented Aug 24, 2022

Agreed - I think it does make sense to add more caching strategies. (Though note that you can also implement your own by setting initialCache: !hasEnoughTimePassed() or equivalent.)

@danielroe
Copy link
Member

Created two issues to follow:

Feel free to open a new one if you think there's another one to track.

@some-user123
Copy link

some-user123 commented Aug 24, 2022

Minimal reproduction for the above, using script setup for readability: https://stackblitz.com/edit/github-5jhffm

Should I open a separate issue for that?

Edit: Created #14695.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants
@Allypost @Aareksio @misaon @danielroe @AndersNielsen85 @some-user123 and others