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

feat: Plugin error handling #1742

Merged
merged 22 commits into from
Mar 15, 2022
Merged

feat: Plugin error handling #1742

merged 22 commits into from
Mar 15, 2022

Conversation

jhildenbiddle
Copy link
Member

@jhildenbiddle jhildenbiddle commented Feb 1, 2022

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.

CleanShot 2022-03-14 at 15 33 35@2x

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.

CleanShot 2022-03-14 at 15 33 01@2x

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.

CleanShot 2022-03-14 at 15 33 21@2x

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

CleanShot 2022-03-14 at 15 33 35@2x

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:

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

@vercel
Copy link

vercel bot commented Feb 1, 2022

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
Copy link

codesandbox-ci bot commented Feb 1, 2022

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

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

@trusktr trusktr left a comment

Choose a reason for hiding this comment

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

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 jhildenbiddle self-assigned this Feb 3, 2022
@jhildenbiddle jhildenbiddle modified the milestone: 4.x Feb 3, 2022
@jhildenbiddle jhildenbiddle changed the title Plugin error handling feat: Plugin error handling Feb 4, 2022
@jhildenbiddle
Copy link
Member Author

jhildenbiddle commented Mar 13, 2022

@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.

@jhildenbiddle jhildenbiddle requested review from a team and trusktr and removed request for trusktr March 13, 2022 21:38
@jhildenbiddle jhildenbiddle marked this pull request as ready for review March 13, 2022 21:38
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.

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

@jhildenbiddle
Copy link
Member Author

jhildenbiddle commented Mar 14, 2022

@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

CleanShot 2022-03-14 at 13 08 49@2x

Safari 15.3

CleanShot 2022-03-14 at 13 07 03@2x

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:

  1. How will people know when an error has occurred if the console isn't open?
  2. 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!

@jhildenbiddle jhildenbiddle requested review from Koooooo-7 and a team March 14, 2022 20:00
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.

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.

Thank you for your contribution.

@jhildenbiddle jhildenbiddle merged commit 63b2535 into develop Mar 15, 2022
@jhildenbiddle jhildenbiddle deleted the plugin-error-handling branch March 15, 2022 05:18
@jhildenbiddle
Copy link
Member Author

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

Koooooo-7 added a commit that referenced this pull request Oct 26, 2022
* fix: packages/docsify-server-renderer/package.json & packages/docsify-server-renderer/package-lock.json to reduce vulnerabilities (#1715)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-DOCSIFY-1090577

* fix: correctly fix missing +1, -1 (#1722)

* Update LICENSE

* mention that SSR is experimental and incomplete, prevent people from using it thinking it is ready for prime time

* refactor: Update test environments and lint configuration (#1736)

* Update test environments and lint configuration

Update Jest (unit + integration) and Playwright (e2e) test environments. Includes stability improvements for e2e tests using newer, more stable methods per the Playwright docs.

- Update Jest 26 => 27
- Update Jest-related libs (babel parser)
- Update Playwright 1.8 => Playwright Test 1.18
- Update GitHub CI (action versions, job parallelization, and matrices)
- Update ESLint 5 => 8
- Update ESLint-related libs (parser, prettier, Jest, Playwright)
- Fix test failures on M1-based Macs
- Fix e2e stability issues by replacing PW $ method calls
- Fix ESLint errors
- Fix incorrect CI flag on Jest runs (-ci => --ci)
- Refactor e2e test runner from Jest to Playwright Test
- Refactor e2e test files for Playwright Test
- Refactor fix-lint script name to lint:fix for consistency
- Refactor npm scripts order for readability
- Remove unnecessary configs and libs
- Remove example image snapshots

* chore: bump node-fetch in /packages/docsify-server-renderer (#1738)

Bumps [node-fetch](https://github.com/node-fetch/node-fetch) from 2.6.6 to 2.6.7.
- [Release notes](https://github.com/node-fetch/node-fetch/releases)
- [Commits](node-fetch/node-fetch@v2.6.6...v2.6.7)

---
updated-dependencies:
- dependency-name: node-fetch
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* docs: update readme (#1740)

* fix: Coverpage when content > viewport height (#1744)

* fix: .gitignore paths
* fix: Coverpage when content > viewport height

fix #381

* chore: sync emojis (#1745)

* fix: Legacy bugs (styles, site plugin error, and dev server error) (#1743)

* Add try/catch w/ error message to plugin calls

* Update lifecycle.js

* Update lifecycle.js

* Fix docsify-plugin-carbon error

* Fix ESLint errors

* Simplify conditional JS loading

* Fix styles in legacy browser w/o CSS var support

* Fix gitignore paths

* Fix BrowserSync IE error

* Fix search field presentation in IE11

- Removed fixed height and allow element to size naturally via font-size and padding
- Remove default "x" rendered on IE input fields

* Revert "Update lifecycle.js"

This reverts commit 2a58be6.

* Revert "Update lifecycle.js"

This reverts commit 67c5410.

* Revert "Add try/catch w/ error message to plugin calls"

This reverts commit 631e924.

* Fix docsify-plugin-carbon error & ESLint errors

Co-authored-by: 沈唁 <[email protected]>

* fix: package.json & package-lock.json to reduce vulnerabilities (#1756)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-PRISMJS-2404333

* chore: bump follow-redirects from 1.14.7 to 1.14.9 (#1757)

Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.14.7 to 1.14.9.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.14.7...v1.14.9)

---
updated-dependencies:
- dependency-name: follow-redirects
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore: bump prismjs in /packages/docsify-server-renderer (#1760)

Bumps [prismjs](https://github.com/PrismJS/prism) from 1.26.0 to 1.27.0.
- [Release notes](https://github.com/PrismJS/prism/releases)
- [Changelog](https://github.com/PrismJS/prism/blob/master/CHANGELOG.md)
- [Commits](PrismJS/prism@v1.26.0...v1.27.0)

---
updated-dependencies:
- dependency-name: prismjs
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: Native emoji w/ image-based fallbacks and improved parsing (#1746)

* Render native emoji with image fallback

Fix #779

* Deprecate emoji plugin

* Add emoji tests

* Remove console.log statement

* Fix emoji image alt attribute

* Set nativeEmoji to false by default (non-breaking)

* Fix parsing emoji in HTML comments and script tags

* Add nativeEmoji and update noEmoji details

* Add Emoji plugin deprecation notice

* Fix ESLint issues

* Create build:emoji task

- Auto-generate emoji data from GitHub API
- Auto-generate emoji markdown for website
- Add emoji page to navigation

* Fix rendering of GitHub emoji without unicode

* Adjust and match size of native and image emoji

* Update emoji test snapshot

* Update docs test snapshot

* Fix ci/codesandbox error

* Update native emoji font-stack

* Fix rendering of native multi-character emoji

* Kick GitHub Workflow

* Replace rollup’s uglify plugin with terser

* Switch “npm ci” instead of “npm i” for stability

* Change emoji data from default to named export

* Revert "Replace rollup’s uglify plugin with terser"

This reverts commit 7ba8513.

* Revert "Switch “npm ci” instead of “npm i” for stability"

This reverts commit d52b476.

* Revert "Change emoji data from default to named export"

This reverts commit 3f2dd46.

* Specify codesandbox template and node version

* Update codesandbox config

* Revert "Revert "Replace rollup’s uglify plugin with terser""

This reverts commit e06fed4.

* Revert "Revert "Revert "Replace rollup’s uglify plugin with terser"""

This reverts commit 27d4952.

* Update codesandbox config

* Revert "Update codesandbox config"

This reverts commit 5120dd2.

* Fix codesandbox uglify error

* Emoji docs tweaks

* Restore and update emoji plugin code

* Restore and update emoji plugin docs

* Prettier updates

* Match lowercase shortcodes only

Co-authored-by: Koy Zhuang <[email protected]>

* feat: Emoji build (#1766)

* Fix incorrect file name

* Improve build

- Display emoji API URL
- Display number of emoji entries retrieved from API
- Distinguish between creating and updating files
- Catch and display errors (gracefully fail for offline work)
- Add “DO NOT EDIT” comment to generated output

* Add emoji to automated build

* Remove emoji plugin from dev index.html

* chore: Remove dompurify (#1490)

* chore: update develop branch preview link (#1772)

* feat: Plugin error handling (#1742)

* Fix: ready/doneEach order with async afterEach (#1770)

* docs: Update configuration options (#1773)

* docs: Minor fixes to plugin docs (#1774)

* chore: update vercel link (#1775)

* chore: update vercel link

* chore: update vercel logo (#1776)

* chore: update vercel logo

* chore: update vercel logo

* chore: remove vercel link form github pages

* chore: update style

* chore: update readme

* chore: update readme

* chore: update readme

* chore: Update CONTRIBUTING.md (#1782)

Update URL of "How to Contribute to an Open Source Project on GitHub" link. The old one send the user to a 404 page where egghead suggest the new and correct URL. So, this change send the user direct to the correct URL.

* chore: bump minimist from 1.2.5 to 1.2.6 (#1787)

Bumps [minimist](https://github.com/substack/minimist) from 1.2.5 to 1.2.6.
- [Release notes](https://github.com/substack/minimist/releases)
- [Commits](https://github.com/substack/minimist/compare/1.2.5...1.2.6)

---
updated-dependencies:
- dependency-name: minimist
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Virtual Routes Support (#1799)

* add first test

* new VirtualRoutes mixin that handles routes. fetch tries to use VirtualRoutes. default config updated

* cover all basic use cases

* regex matching in routes

* covered all virtual routes tests

* added hack to fix config test on firefox

* removed formatting regex matches into string routes

* added support for "next" function

* added docs

* navigate now supports both hash and history routerModes

* waiting for networkidle in navigateToRoute helper

* promiseless implementation

* remove firefox workaround from catchPluginErrors test, since we no longer use promises

* updated docs

* updated docs for "alias" as well

* minor rephrasing

* removed non-legacy code from exact-match; updated navigateToRoute helper to infer router mode from page

* moved endsWith from router utils to general utils; added startsWith util; refactored makeExactMatcher to use both

* updated docs per feedback

* moved navigateToRoute helper into the virtual-routes test file

* moved navigateToRoute to top of file

* updated docs per pr comments

* docs: update the name for "Simplified Chinese" (#1811)

* update the name for "Simplified Chinese"

* update the name for "Simplified Chinese"

* update the name for "Simplified Chinese"

* fix: cornerExternalLinkTarget config. (#1814)

* Improve README.md sentence (#1817)

* chore: bump jpeg-js from 0.4.3 to 0.4.4 (#1820)

Bumps [jpeg-js](https://github.com/eugeneware/jpeg-js) from 0.4.3 to 0.4.4.
- [Release notes](https://github.com/eugeneware/jpeg-js/releases)
- [Commits](jpeg-js/jpeg-js@v0.4.3...v0.4.4)

---
updated-dependencies:
- dependency-name: jpeg-js
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore: bump parse-url from 6.0.0 to 6.0.2 (#1833)

Bumps [parse-url](https://github.com/IonicaBizau/parse-url) from 6.0.0 to 6.0.2.
- [Release notes](https://github.com/IonicaBizau/parse-url/releases)
- [Commits](https://github.com/IonicaBizau/parse-url/commits)

---
updated-dependencies:
- dependency-name: parse-url
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Docs: Fix plugin insertion order in docs (#1850)

* fix: Ignore emoji shorthand codes in URIs (#1847)

* fix: Ignore emoji shorthand codes in URIs

Fixes: #1823

* test: Add test for emoji in anchor body

* fix: Handle support for URIs used in additional contexts

Examples:

- Without explicit scheme (i.e. starting with `//`)
- In single and double quote strings
- Within unquoted HTML tag attributes
- In css `url()` values

Co-authored-by: John Hildenbiddle <[email protected]>

* fix: fix docsify-ignore in seach title. (#1872)

* fix: fix search with no content. (#1878)

* docs: Update GitHub default branch from to 'main' (#1883)

* chore: upgrade caniuse-lit. (#1879)

* chore: bump trim-newlines and lerna (#1895)

Bumps [trim-newlines](https://github.com/sindresorhus/trim-newlines) and [lerna](https://github.com/lerna/lerna/tree/HEAD/core/lerna). These dependencies needed to be updated together.

Updates `trim-newlines` from 1.0.0 to 3.0.1
- [Release notes](https://github.com/sindresorhus/trim-newlines/releases)
- [Commits](https://github.com/sindresorhus/trim-newlines/commits)

Updates `lerna` from 3.22.1 to 5.5.1
- [Release notes](https://github.com/lerna/lerna/releases)
- [Changelog](https://github.com/lerna/lerna/blob/main/core/lerna/CHANGELOG.md)
- [Commits](https://github.com/lerna/lerna/commits/v5.5.1/core/lerna)

---
updated-dependencies:
- dependency-name: trim-newlines
  dependency-type: indirect
- dependency-name: lerna
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: filter null node first. (#1909)

* [build]: 4.12.3

* [build] 4.12.4

* chore: add changelog 4.12.4

* update: update dependencies.

* fix: fix test.

* fix: upgrade dependencies.

* [build] 4.13.0

* chore: add changelog 4.13.0

* chore: add changelog v4.13.0

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Snyk bot <[email protected]>
Co-authored-by: ChoKaPeek <[email protected]>
Co-authored-by: Joe Pea <[email protected]>
Co-authored-by: John Hildenbiddle <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: 沈唁 <[email protected]>
Co-authored-by: Bernal I. Hernández <[email protected]>
Co-authored-by: Roy Sommer <[email protected]>
Co-authored-by: Xavi Lee <[email protected]>
Co-authored-by: Shaun Bharat <[email protected]>
Co-authored-by: Soc Sieng <[email protected]>
Co-authored-by: Mike Mwanje <[email protected]>
@Koooooo-7 Koooooo-7 mentioned this pull request Oct 26, 2022
Koooooo-7 pushed a commit that referenced this pull request Oct 26, 2022
Koooooo-7 added a commit that referenced this pull request Oct 26, 2022
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 this pull request may close these issues.

Handle uncaught plugin errors to prevent live sites from breaking
4 participants