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

fix(vue-app): handle server-side routing errors #7801

Merged
merged 12 commits into from Aug 4, 2020
Merged

fix(vue-app): handle server-side routing errors #7801

merged 12 commits into from Aug 4, 2020

Conversation

matthieusieben
Copy link

@matthieusieben matthieusieben commented Jul 28, 2020

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Resolves: #7798

https://github.com/nuxt/nuxt.js/blob/4caffb04ab9309fb334f68739c61b73f8c325a05/packages/vue-app/template/index.js#L248-L268

This introduces a fix for 2 use cases:

  • The user uses next(err) in a router guard. This will generate a 500 error.
  • The user uses next(false) in a router guard. This will:
    • Either render any error provided through context.error()
    • Or generate a 404 (by default)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2020

Codecov Report

Merging #7801 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #7801   +/-   ##
=======================================
  Coverage   68.89%   68.89%           
=======================================
  Files          90       90           
  Lines        3838     3838           
  Branches     1038     1038           
=======================================
  Hits         2644     2644           
  Misses        970      970           
  Partials      224      224           
Flag Coverage Δ
#unittests 68.89% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab30bbc...422d3ac. Read the comment docs.

@pi0 pi0 requested a review from clarkdo July 29, 2020 09:56
@matthieusieben matthieusieben changed the title fix: handle server-side routing error fix(route): handle server-side routing error Jul 29, 2020
@matthieusieben
Copy link
Author

These changes will imply that users should never do next(err) (or they will get a 500) but do error(err); next(false) instead.

Note that in the browser, nothing will happen if a user does next(err) except from displaying an error in the console.

packages/vue-app/template/index.js Outdated Show resolved Hide resolved
packages/vue-app/template/index.js Outdated Show resolved Hide resolved
@matthieusieben matthieusieben requested a review from pi0 July 31, 2020 08:04
@pi0 pi0 changed the title fix(route): handle server-side routing error fix(vue-app): handle server-side routing errors Aug 3, 2020
@pi0 pi0 merged commit 128c974 into nuxt:dev Aug 4, 2020
@pi0
Copy link
Member

pi0 commented Aug 4, 2020

Thanks for PR @matthieusieben ❤️

@pi0 pi0 mentioned this pull request Aug 4, 2020
@danielroe danielroe added the 2.x label Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

router guard error causes SSR request to never complete
4 participants