feat: Plugin error handling #1742

jhildenbiddle · 2022-02-01T17:59:16Z

Summary

Fix #1741 (see for details)

What kind of change does this PR introduce?

Add new catchPluginErrors configuration option to enable/disable handling uncaught plugin errors (default: true). This feature prevents uncaught plugin errors from halting JavaScript execution and breaking Docsify sites.
Add e2e tests for catchPluginErrors configuration option
Add e2e tests for all plugin hooks (data handling and invocation order)
Update Write a Plugin new new Setup, Template, Lifecycle Hooks, and Tips sections (see Vercel preview below)

Screenshots

Assuming the following plugin code:

window.$docsify = {
  // ...
  plugins: [
    function (hook, vm) {
      hook.beforeEach(function (content) {
        console.log('Plugin 1');
        console.log(['fail', 'success'].at(1)); // <= ERROR: at() not supported by Safari
      });
    },
    function (hook, vm) {
      hook.beforeEach(function (content) {
        console.log('Plugin 2');
        console.log(['fail', 'success'][1]);
      });
    }
  ]
}

Before

Safari 15.3: Uncaught "Plugin 1" error halts JavaScript execution, breaking docsify site.

After

Safari 15.3 w/ catchPluginErrors: true (default): Uncaught "Plugin 1" error is caught by docsify and logged to the console, allowing Docsify to continue with "Plugin 2" and rendering site content.

Note that error details are provided in the console as they were before. Site owners and plugin authors that use the console to troubleshoot and debug can continue to do so. Safari collapses these details by default. Chrome does not.

Safari 15.3 w/ catchPluginErrors: false: Same as "Before".

For any code change,

Related documentation has been updated if needed
Related tests have been updated or tests have been added

Does this PR introduce a breaking change? (check one)

Yes
No

If yes, please describe the impact and migration path for existing applications:

Related issue, if any:

#1741
#1209

Tested in the following browsers:

vercel · 2022-02-01T17:59:19Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/docsify-core/docsify-preview/CdsP8nNhoRDzC6NAqWZAPvGxFH4i
✅ Preview: https://docsify-preview-git-plugin-error-handling-docsify-core.vercel.app

codesandbox-ci · 2022-02-01T18:00:30Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 650870c:

Sandbox	Source
docsify-template	Configuration

trusktr

Is the error navigable the same way as an uncaught error, in console?

Now that errors are caught, they will not be pausable by default in debugger, and the pause on caught errors option generally pauses too much that it can become inconvenient.

Personally I would prefer to see my actual errors in console, and in the console I can clearly see which plugin I wrote bad code in.

The error stack trace of an error that is shown in console tells us exactly where the error came from (hence we already know if it comes from a plugin)

jhildenbiddle · 2022-03-13T21:21:59Z

@trusktr --

Details available in #1741...

Is the error navigable the same way as an uncaught error, in console?

Yes.

Now that errors are caught, they will not be pausable by default in debugger, and the pause on caught errors option generally pauses too much that it can become inconvenient.

Personally I would prefer to see my actual errors in console, and in the console I can clearly see which plugin I wrote bad code in.

Set catchPluginErrors to false during plugin development if you wish to use a debugger as you do today.

The error stack trace of an error that is shown in console tells us exactly where the error came from (hence we already know if it comes from a plugin)

Still there. See screenshots above or in #1741.

Koooooo-7

should we console all hooks directly or we should have a switch to open this console output? i.e.
debugEnable: true.

jhildenbiddle · 2022-03-14T20:00:49Z

@Koooooo-7 --

should we console all hooks directly or we should have a switch to open this console output? i.e.debugEnable: true.

I'm not sure I understand the question. It sounds like you're asking if we should add a new configuration option that either A) prevents uncaught plugin errors from being displayed in the console or B) opens the console automatically when an error occurs (presumably to make people aware that the error occurred).

I don't think we should consider either option.

For option "A": Errors should continue to be sent to the console as they are today because the console is where developers and end-users expect to find error-related details. This PR does not change that, and that is by design. Developers and end-users that use the console to debug and/or troubleshoot can continue to do so exactly as they do today. Site visitors that don't know or care about the console are free to ignore it exactly as they do today.

It's worth mentioning that if a plugin author or end user really wanted to suppress error messages in the console, they can do so already by implementing their own try/catch block(s) and/or using the console message filters provided by the browser:

Chrome 99

Safari 15.3

For option "B": I don't think it is possible to programmatically open the browser console, but even if was we should not assume that every site visitor 4) needs to be notified that a plugin error has occurred, 2) knows what the console is, 3) will understand why the console is being displayed, or 4) knows how to dismiss the console. The console is there for the people that need it, but it is not intended to be used as a general-purpose notification tool.

All that being said, your question does hint at two separate-but-related topics:

How will people know when an error has occurred if the console isn't open?
What should happen when an error occurs?

These are valid questions, but I believe they are outside the scope of this PR. If we want to address them, let's create a new issue where we can discuss the details. I have some ideas regarding a new error-related hook that would address both of these concerns and provide some interesting plugin opportunities.

In the meantime, my hope is that we can merge this PR and discuss other plugin-related enhancements elsewhere.

Thanks!

Koooooo-7

LGTM.

sy-records

LGTM.

Thank you for your contribution.

jhildenbiddle · 2022-03-15T05:19:14Z

Thanks @Koooooo-7 and @sy-records for the review! 🥳

jhildenbiddle added 2 commits Feb 1, 2022

Add try/catch w/ error message to plugin calls

631e924

Merge branch 'develop' into plugin-error-handling

cfbcd81

jhildenbiddle added docs related to the documentation of docsify itself enhancement plugin related to plugin stuff. labels Feb 2, 2022

jhildenbiddle added this to In progress in 4.x via automation Feb 2, 2022

trusktr reviewed Feb 2, 2022

View changes

jhildenbiddle added 2 commits Feb 3, 2022

Update lifecycle.js

67c5410

Merge branch 'develop' into plugin-error-handling

6cb9a58

vercel bot deployed to Preview Feb 3, 2022 View deployment

Update lifecycle.js

2a58be6

vercel bot deployed to Preview Feb 3, 2022 View deployment

jhildenbiddle self-assigned this Feb 3, 2022

jhildenbiddle modified the milestone: 4.x Feb 3, 2022

jhildenbiddle changed the title ~~Plugin error handling~~ feat: Plugin error handling Feb 4, 2022

Merge branch 'develop' into plugin-error-handling

e276411

vercel bot deployed to Preview Feb 5, 2022 View deployment

Merge branch 'develop' into plugin-error-handling

0a9670e

vercel bot deployed to Preview Mar 7, 2022 View deployment

jhildenbiddle added 2 commits Mar 8, 2022

Add try/catch to plugin pre-hooks

97b91fa

Update error message

b808363

vercel bot deployed to Preview Mar 8, 2022 View deployment

Merge branch 'develop' into plugin-error-handling

2a39cea

vercel bot deployed to Preview Mar 8, 2022 View deployment

Add catchPluginErrors option

c2bf5b9

jhildenbiddle mentioned this pull request Mar 10, 2022

Handle uncaught plugin errors to prevent live sites from breaking #1741

Closed

Fix async beforeEach calls

99c9929

vercel bot deployed to Preview Mar 11, 2022 View deployment

Remove unnecesary code block

bf72293

vercel bot deployed to Preview Mar 11, 2022 View deployment

jhildenbiddle added 5 commits Mar 11, 2022

Fix handling of hook return value

c3c534c

Update template to handle no config scenario

7ae61ea

Update async hook text and examples

89451d7

Add tests for catchPluginErrors option

dd40e80

Add tests for plugin system

a031405

vercel bot deployed to Preview Mar 11, 2022 View deployment

Update code samples

70011e2

jhildenbiddle requested review from a team and trusktr and removed request for trusktr Mar 13, 2022

vercel bot deployed to Preview Mar 13, 2022 View deployment

jhildenbiddle marked this pull request as ready for review Mar 13, 2022

Koooooo-7 reviewed Mar 14, 2022

View changes

jhildenbiddle requested review from Koooooo-7 and a team Mar 14, 2022

Merge branch 'develop' into plugin-error-handling

650870c

vercel bot deployed to Preview Mar 15, 2022 View deployment

Koooooo-7 approved these changes Mar 15, 2022

View changes

sy-records approved these changes Mar 15, 2022

View changes

jhildenbiddle merged commit 63b2535 into develop Mar 15, 2022
9 checks passed

4.x automation moved this from In progress to Done Mar 15, 2022

jhildenbiddle deleted the plugin-error-handling branch Mar 15, 2022

jhildenbiddle mentioned this pull request Mar 18, 2022

Add beforeEmbed and afterEmbed hook #1571

Closed

9 tasks

Aug	SEP	Jan
	04
2021	2022	2023

feat: Plugin error handling #1742

feat: Plugin error handling #1742

jhildenbiddle commented Feb 1, 2022 •

edited

vercel bot commented Feb 1, 2022 •

edited

codesandbox-ci bot commented Feb 1, 2022 •

edited

trusktr left a comment •

edited

jhildenbiddle commented Mar 13, 2022 •

edited

Koooooo-7 left a comment

jhildenbiddle commented Mar 14, 2022 •

edited

Koooooo-7 left a comment

sy-records left a comment

jhildenbiddle commented Mar 15, 2022

feat: Plugin error handling #1742

feat: Plugin error handling #1742

Conversation

jhildenbiddle commented Feb 1, 2022 • edited

Summary

What kind of change does this PR introduce?

Screenshots

Before

After

For any code change,

Does this PR introduce a breaking change? (check one)

Related issue, if any:

Tested in the following browsers:

vercel bot commented Feb 1, 2022 • edited

codesandbox-ci bot commented Feb 1, 2022 • edited

trusktr left a comment • edited

jhildenbiddle commented Mar 13, 2022 • edited

Koooooo-7 left a comment

jhildenbiddle commented Mar 14, 2022 • edited

Koooooo-7 left a comment

sy-records left a comment

jhildenbiddle commented Mar 15, 2022

jhildenbiddle commented Feb 1, 2022 •

edited

vercel bot commented Feb 1, 2022 •

edited

codesandbox-ci bot commented Feb 1, 2022 •

edited

trusktr left a comment •

edited

jhildenbiddle commented Mar 13, 2022 •

edited

jhildenbiddle commented Mar 14, 2022 •

edited