Skip to content

Feat: Improved error handling and response status availability #2303

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

Merged
merged 8 commits into from
Nov 17, 2023

Conversation

jhildenbiddle
Copy link
Member

@jhildenbiddle jhildenbiddle commented Nov 14, 2023

Summary

  • Add response status to internal route object
  • Expose response status to plugin hooks to allow for custom error handling.
  • Update default error page content with error status and status text (e.g. "401 - Not Authorized", "500 - Internal Server Error", etc.) instead of displaying "404 - Not Found" for all errors.
  • Fix issue where initial site render was incomplete when content fetch failed (e.g., no sidebar, plugins halted, etc.)
  • Fix issue where empty markdown pages/routes were handled as 404 errors.
  • Add tests to verify default error handling
  • Add tests to verify notFoundPage error handling
  • Add tests to verify response data availability via plugins

See #2294 for details.

Screenshots

Custom error handling via plugins (example)

CleanShot 2023-11-16 at 21 57 43@2x

window.$docsify = {
  // ...
  plugins: [
    function (hook, vm) {
      hook.beforeEach(html => {
        const { file, response } = vm.route;
        const { ok, status, statusText } = response;

        if (ok === false) {
          return [
            `# Oops!`,
            `A **${status} - ${statusText}** error occurred while requesting **${vm.route.file}**`,
          ].join('\n\n');
        }
      });
    },
  ],
}

Default error content updates (500 error)

Before: Displays inaccurate error information (shows 404 but should be 500)

CleanShot 2023-11-16 at 19 03 25@2x

After: Displays accurate error information

CleanShot 2023-11-16 at 19 03 32@2x


Initial render with content load error

Before: No sidebar, plugins halted

CleanShot 2023-11-16 at 18 22 49@2x

After: Sidebar rendered, plugins complete

CleanShot 2023-11-16 at 18 22 56@2x


Rendering empty files/routes

Before: Empty files incorrectly treated as 404 errors

CleanShot 2023-11-16 at 19 10 49@2x

After: Empty files rendered properly

CleanShot 2023-11-16 at 19 11 01@2x


Related issue, if any:

#2294

What kind of change does this PR introduce?

Feature

For any code change,

  • Related tests have been added or updated, if needed

Does this PR introduce a breaking change?

No

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari
  • Edge

Copy link

vercel bot commented Nov 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docsify-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 17, 2023 1:15am

@jhildenbiddle jhildenbiddle marked this pull request as ready for review November 14, 2023 20:42
sy-records
sy-records previously approved these changes Nov 15, 2023
Copy link
Member

@sy-records sy-records left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@sy-records sy-records requested a review from a team November 15, 2023 00:11
@jhildenbiddle jhildenbiddle linked an issue Nov 16, 2023 that may be closed by this pull request
@jhildenbiddle jhildenbiddle changed the title Feat: Expose response status internally and to plugins Feat: Improved error handling and response status availability Nov 17, 2023
Copy link
Member

@Koooooo-7 Koooooo-7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, that really a customize handler hook enhancement.

@jhildenbiddle jhildenbiddle merged commit dac8e59 into develop Nov 17, 2023
@jhildenbiddle jhildenbiddle deleted the 2294-route-response branch November 17, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Add response status to route object and plugins
3 participants