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

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

Closed
jhildenbiddle opened this issue Feb 1, 2022 · 2 comments · Fixed by #1742
Closed

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

jhildenbiddle opened this issue Feb 1, 2022 · 2 comments · Fixed by #1742
Assignees
Labels
docs related to the documentation of docsify itself enhancement plugin related to plugin stuff.

Comments

@jhildenbiddle
Copy link
Member

jhildenbiddle commented Feb 1, 2022

Feature request

Docsify should handle uncaught plugin errors to prevent JavaScript execution from halting and breaking docsify sites.

What problem does this feature solve?

Docsify sites are rendered on the client using JavaScript. When a plugin error occurs, JavaScript execution is halted. The result for site visitors is a "broken" site typically rending only a "Loading..." message or an unresponsive UI (depending on when the plugin was invoked and what type of error occurs).

The problem with uncaught plugin errors is that they often go unnoticed by site owners and plugin authors and end up breaking live sites. For example, here are just a few uncaught plugin error issues that I have personally worked on:

Every issue above was the result of a plugin author using an unsupported method or ES6+ syntax, and each one prevented docsify sites from rendering properly in IE10/11. These bugs went undetected until I started testing docsify-themeable with various plugins in all supported platforms (which at the time included IE10 as well as IE11). If Docsify handled those uncaught plugin errors, the sites would have rendered properly (albeit without the expected plugin functionality).

The examples above are all "legacy" browser related, but this is not a legacy browser issue. The same issue exists for evergreen browsers since browser vendors choose if and when they will adopt new ES202X methods and syntax changes. For example, consider ES2022's Array.prototype.at() feature which is currently supported by all evergreen browsers except Safari:

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

Chrome 99:

CleanShot 2022-03-09 at 15 09 57@2x

Safari 15.3 (latest):

CleanShot 2022-03-09 at 15 10 06@2x

This seems like an obvious error that one might expect to be caught by either the site owner or the plugin author, but it is not that simple:

  • What if the site owner and/or plugin author does not have the knowledge or resources to test on all supported platforms?
  • What if the plugin error is caused by a conflict with another plugin?
  • What if the plugin error occurs only when a specific configuration option is set?
  • What if the plugin error occurs only when an infrequent event occurs (e.g., malformed markdown or HTML, local storage limits exceeded, interaction on mobile device in landscape mode, etc.)?

These are not theoretical examples. We have real-world examples of each of them: #1057, #1392, #1526, #1538, ... etc. Obvious bugs are easy to detect, but not all bugs are obvious.

Ideally, plugin authors would conscientiously handle their own errors and thoroughly test on all platforms supported by Docsify. We know from years of docsify plugin development that this doesn't happen. The best we can do to prevent these issues is to make Docsify more resilient when possible and provide some best practiceson our Write a Plugin section.

Note: Asynchronous code will require plugin authors to implement their own try/catch/finally blocks directly in their code. An update to the official documentation that explains this would be helpful.

hook.afterEach(function (html, next) {
  try {
    // Async tasks...
    console.log("Plugin 1");
    console.log(["fail", "success"].at(1));
  } catch (err) {
    // ...
  } finally {
    next();
  }
});

What does the proposed API look like?

A catchPluginErrors configuration option can be added to allow uncaught plugin errors to throw as usual. This may be useful for docsify contributors, plugin authors, and site owners who rely on uncaught errors for their development or site monitoring tools.

  • Type: boolean
  • Default: true
window.$docsify = {
  catchPluginErrors: true // default
}

How should this be implemented in your opinion?

As a non-breaking change with documentation updated accordingly.

Are you willing to work on this yourself?

Yes

@jhildenbiddle jhildenbiddle self-assigned this Feb 1, 2022
@jhildenbiddle jhildenbiddle added docs related to the documentation of docsify itself enhancement plugin related to plugin stuff. labels Feb 1, 2022
@trusktr
Copy link
Member

trusktr commented Feb 2, 2022

Errors still need to go to console. IMO, authors should catch their own errors in this case, and they should be testing in multiple browsers (just as with any other framework besides Docsify).

I.e. it isn't our responsibility to catch errors from people writing invalid code. That's their problem.

Whoever doesn't know how to write and test working code needs to learn. Catching their errors won't help them, it'll only hide issues from them.

@jhildenbiddle jhildenbiddle changed the title Handle plugin errors to prevent sites from breaking Handle uncaught plugin errors to prevent sites from breaking Mar 9, 2022
@jhildenbiddle jhildenbiddle changed the title Handle uncaught plugin errors to prevent sites from breaking Handle uncaught plugin errors to prevent live sites from breaking Mar 10, 2022
@jhildenbiddle
Copy link
Member Author

jhildenbiddle commented Mar 10, 2022

@trusktr --

I've updated the original post to (hopefully) explain the issue a bit better. It's worth re-reading it before proceeding.

Errors still need to go to console.

Errors will continue to be displayed in the console along with a full stack trace with links to where the bug occurred in the source. Site owners and plugin authors that use the console to troubleshoot and debug can continue to do so.

You brought up a good point about debuggers in your PR comment though. This motivated me to add the catchPluginErrors configuration option. Setting this option to false will allow debuggers to pause on error as they do now.

IMO, authors should catch their own errors in this case, and they should be testing in multiple browsers (just as with any other framework besides Docsify).

They should, but they don't. We know this because we have years of docsify plugin development and usage to prove it.

Whoever doesn't know how to write and test working code needs to learn. Catching their errors won't help them, it'll only hide issues from them.

This isn't about helping developers. This is about improving the experience for docsify site owners and visitors by preventing live sites from breaking.

A system that executes third-party code needs to make reasonable efforts to protect itself from the the potential negative side effects of executing that code. For example, the operating systems we use everyday do this to prevent application errors from crashing the entire system. This is no different. An uncaught plugin error should not crash an entire Docsify site.

Docsify has its own history of site-breaking plugin bugs that were introduced and/or undetected by Docsify's creator and core maintainers (#300, #686, #1337, #1538, etc). I don't think it's fair say that these people "[don't] know how to write and test working code". I assume they tested to the best of their ability or to a level they they felt was sufficient, but that wasn't enough to detect the bug. It happens, and it will continue to happen because bugs are an unavoidable part of software development.

Finally, It's also important to acknowledge that not all Docsify users or plugin authors are developers. Docsify Open Course Starter Kit is just one example of a docsify project created and maintained by a non-developer. The project uses both third-party and custom plugins, any one of which could throw an error that has gone undetected by both the project owner and a plugin author (see "what if's" in the original comment above). The end result would be a broken site for visitors affected by the bug. If Docsify handles the error, the end result will be a working site, albeit without the expected plugin functionality. I believe docsify users want to avoid the former and expect the latter.

I do appreciate the feedback. The catchPluginErrors option is a nice addition that hadn't occurred to me until you mentioned using a debugger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs related to the documentation of docsify itself enhancement plugin related to plugin stuff.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants