Feat: Improved error handling and response status availability (#2303)

* Add response data to route object
* Display error status and description on content fetch error
* Fix issue where initial site render was incomplete on content fetch error
* Fix issue where empty markdown pages/routes were handled as 404 errors
* Fix incorrect `notFoundPage` default value
This commit is contained in:
John Hildenbiddle 2023-11-17 07:27:09 -06:00 committed by GitHub
parent ae71c69e1b
commit dac8e59bb7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 302 additions and 134 deletions

View File

@ -534,7 +534,7 @@ To disable emoji parsing of individual shorthand codes, replace `:` characters w
- Type: `Boolean` | `String` | `Object`
- Default: `false`
Display default "404 - Not found" message:
Display default "404 - Not Found" message:
```js
window.$docsify = {

View File

@ -30,7 +30,7 @@ export default function (vm) {
nativeEmoji: false,
noCompileLinks: [],
noEmoji: false,
notFoundPage: true,
notFoundPage: false,
plugins: [],
relativePath: false,
repo: '',

View File

@ -102,7 +102,8 @@ export function Fetch(Base) {
this.isHTML = /\.html$/g.test(file);
// create a handler that should be called if content was fetched successfully
const contentFetched = (text, opt) => {
const contentFetched = (text, opt, response) => {
this.route.response = response;
this._renderMain(
text,
opt,
@ -111,7 +112,8 @@ export function Fetch(Base) {
};
// and a handler that is called if content failed to fetch
const contentFailedToFetch = _error => {
const contentFailedToFetch = (_error, response) => {
this.route.response = response;
this._fetchFallbackPage(path, qs, cb) || this._fetch404(file, qs, cb);
};

View File

@ -60,10 +60,6 @@ export function Render(Base) {
return isVue2 || isVue3;
};
if (!html) {
html = /* html */ `<h1>404 - Not found</h1>`;
}
if ('Vue' in window) {
const mountedElms = dom
.findAll('.markdown-section > *')
@ -310,8 +306,12 @@ export function Render(Base) {
}
_renderMain(text, opt = {}, next) {
if (!text) {
return this.#renderMain(text);
const { response } = this.route;
// Note: It is possible for the response to be undefined in environments
// where XMLHttpRequest has been modified or mocked
if (response && !response.ok && (!text || response.status !== 404)) {
text = `# ${response.status} - ${response.statusText}`;
}
this.callHook('beforeEach', text, result => {

View File

@ -92,6 +92,7 @@ export class HashHistory extends History {
path,
file: this.getFile(path, true),
query: parseQuery(query),
response: {},
};
}

View File

@ -59,6 +59,7 @@ export class HTML5History extends History {
path,
file: this.getFile(path),
query: parseQuery(query),
response: {},
};
}
}

View File

@ -4,10 +4,10 @@ import progressbar from '../render/progressbar.js';
import { noop } from './core.js';
/** @typedef {{updatedAt: string}} CacheOpt */
/** @typedef {{content: string, opt: CacheOpt}} CacheItem */
/** @typedef {{ok: boolean, status: number, statusText: string}} ResponseStatus */
/** @type {Record<string, CacheItem>} */
const cache = {};
/**
@ -37,10 +37,16 @@ export function get(url, hasBar = false, headers = {}) {
return {
/**
* @param {(text: string, opt: CacheOpt) => void} success
* @param {(event: ProgressEvent<XMLHttpRequestEventTarget>) => void} error
* @param {(text: string, opt: CacheOpt, response: ResponseStatus) => void} success
* @param {(event: ProgressEvent<XMLHttpRequestEventTarget>, response: ResponseStatus) => void} error
*/
then(success, error = noop) {
const getResponseStatus = event => ({
ok: event.target.status >= 200 && event.target.status < 300,
status: event.target.status,
statusText: event.target.statusText,
});
if (hasBar) {
const id = setInterval(
_ =>
@ -57,11 +63,15 @@ export function get(url, hasBar = false, headers = {}) {
});
}
xhr.addEventListener('error', error);
xhr.addEventListener('error', event => {
error(event, getResponseStatus(event));
});
xhr.addEventListener('load', event => {
const target = /** @type {XMLHttpRequest} */ (event.target);
if (target.status >= 400) {
error(event);
error(event, getResponseStatus(event));
} else {
if (typeof target.response !== 'string') {
throw new TypeError('Unsupported content type.');
@ -74,7 +84,7 @@ export function get(url, hasBar = false, headers = {}) {
},
});
success(result.content, result.opt);
success(result.content, result.opt, getResponseStatus(event));
}
});
},

View File

@ -3,64 +3,146 @@ import docsifyInit from '../helpers/docsify-init.js';
import { test, expect } from './fixtures/docsify-init-fixture.js';
test.describe('Configuration options', () => {
test('catchPluginErrors:true (handles uncaught errors)', async ({ page }) => {
let consoleMsg, errorMsg;
test.describe('catchPluginErrors', () => {
test('true (handles uncaught errors)', async ({ page }) => {
let consoleMsg, errorMsg;
page.on('console', msg => (consoleMsg = msg.text()));
page.on('pageerror', err => (errorMsg = err.message));
page.on('console', msg => (consoleMsg = msg.text()));
page.on('pageerror', err => (errorMsg = err.message));
await docsifyInit({
config: {
catchPluginErrors: true,
plugins: [
function (hook, vm) {
hook.init(function () {
fail();
});
hook.beforeEach(markdown => {
return `${markdown}\n\nbeforeEach`;
});
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
await docsifyInit({
config: {
catchPluginErrors: true,
plugins: [
function (hook, vm) {
hook.init(function () {
fail();
});
hook.beforeEach(markdown => {
return `${markdown}\n\nbeforeEach`;
});
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
});
const mainElm = page.locator('#main');
expect(errorMsg).toBeUndefined();
expect(consoleMsg).toContain('Docsify plugin error');
await expect(mainElm).toContainText('Hello World');
await expect(mainElm).toContainText('beforeEach');
});
const mainElm = page.locator('#main');
test('false (throws uncaught errors)', async ({ page }) => {
let consoleMsg, errorMsg;
expect(errorMsg).toBeUndefined();
expect(consoleMsg).toContain('Docsify plugin error');
await expect(mainElm).toContainText('Hello World');
await expect(mainElm).toContainText('beforeEach');
page.on('console', msg => (consoleMsg = msg.text()));
page.on('pageerror', err => (errorMsg = err.message));
await docsifyInit({
config: {
catchPluginErrors: false,
plugins: [
function (hook, vm) {
hook.ready(function () {
fail();
});
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
});
expect(consoleMsg).toBeUndefined();
expect(errorMsg).toContain('fail');
});
});
test('catchPluginErrors:false (throws uncaught errors)', async ({ page }) => {
let consoleMsg, errorMsg;
test.describe('notFoundPage', () => {
test.describe('renders default error content', () => {
test.beforeEach(async ({ page }) => {
await page.route('README.md', async route => {
await route.fulfill({
status: 500,
});
});
});
page.on('console', msg => (consoleMsg = msg.text()));
page.on('pageerror', err => (errorMsg = err.message));
await docsifyInit({
config: {
catchPluginErrors: false,
plugins: [
function (hook, vm) {
hook.ready(function () {
fail();
});
test('false', async ({ page }) => {
await docsifyInit({
config: {
notFoundPage: false,
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
});
await expect(page.locator('#main')).toContainText('500');
});
test('true with non-404 error', async ({ page }) => {
await docsifyInit({
config: {
notFoundPage: true,
},
routes: {
'_404.md': '',
},
});
await expect(page.locator('#main')).toContainText('500');
});
test('string with non-404 error', async ({ page }) => {
await docsifyInit({
config: {
notFoundPage: '404.md',
},
routes: {
'404.md': '',
},
});
await expect(page.locator('#main')).toContainText('500');
});
});
expect(consoleMsg).toBeUndefined();
expect(errorMsg).toContain('fail');
test('true: renders _404.md page', async ({ page }) => {
const expectText = 'Pass';
await docsifyInit({
config: {
notFoundPage: true,
},
routes: {
'_404.md': expectText,
},
});
await page.evaluate(() => (window.location.hash = '#/fail'));
await expect(page.locator('#main')).toContainText(expectText);
});
test('string: renders specified 404 page', async ({ page }) => {
const expectText = 'Pass';
await docsifyInit({
config: {
notFoundPage: '404.md',
},
routes: {
'404.md': expectText,
},
});
await page.evaluate(() => (window.location.hash = '#/fail'));
await expect(page.locator('#main')).toContainText(expectText);
});
});
});

View File

@ -1,4 +1,5 @@
import docsifyInit from '../helpers/docsify-init.js';
import { waitForFunction } from '../helpers/wait-for.js';
import { test, expect } from './fixtures/docsify-init-fixture.js';
test.describe('Plugins', () => {
@ -72,84 +73,155 @@ test.describe('Plugins', () => {
expect(consoleMsgs).toEqual(expectedMsgs);
});
test('beforeEach() return value', async ({ page }) => {
await docsifyInit({
config: {
plugins: [
function (hook, vm) {
hook.beforeEach(markdown => {
return 'beforeEach';
});
},
],
},
// _logHTML: true,
test.describe('beforeEach()', () => {
test('return value', async ({ page }) => {
await docsifyInit({
config: {
plugins: [
function (hook, vm) {
hook.beforeEach(markdown => {
return 'beforeEach';
});
},
],
},
// _logHTML: true,
});
await expect(page.locator('#main')).toContainText('beforeEach');
});
await expect(page.locator('#main')).toContainText('beforeEach');
test('async return value', async ({ page }) => {
await docsifyInit({
config: {
plugins: [
function (hook, vm) {
hook.beforeEach((markdown, next) => {
setTimeout(() => {
next('beforeEach');
}, 100);
});
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
});
await expect(page.locator('#main')).toContainText('beforeEach');
});
});
test('beforeEach() async return value', async ({ page }) => {
await docsifyInit({
config: {
plugins: [
function (hook, vm) {
hook.beforeEach((markdown, next) => {
setTimeout(() => {
next('beforeEach');
}, 100);
});
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
test.describe('afterEach()', () => {
test('return value', async ({ page }) => {
await docsifyInit({
config: {
plugins: [
function (hook, vm) {
hook.afterEach(html => {
return '<p>afterEach</p>';
});
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
});
await expect(page.locator('#main')).toContainText('afterEach');
});
await expect(page.locator('#main')).toContainText('beforeEach');
test('async return value', async ({ page }) => {
await docsifyInit({
config: {
plugins: [
function (hook, vm) {
hook.afterEach((html, next) => {
setTimeout(() => {
next('<p>afterEach</p>');
}, 100);
});
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
});
await expect(page.locator('#main')).toContainText('afterEach');
});
});
test('afterEach() return value', async ({ page }) => {
await docsifyInit({
config: {
plugins: [
function (hook, vm) {
hook.afterEach(html => {
return '<p>afterEach</p>';
});
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
test.describe('route data accessible to plugins', () => {
let routeData = null;
test.beforeEach(async ({ page }) => {
// Store route data set via plugin hook (below)
page.on('console', async msg => {
for (const arg of msg.args()) {
const val = await arg.jsonValue();
const obj = JSON.parse(val);
obj.response && (routeData = obj);
}
});
});
await expect(page.locator('#main')).toContainText('afterEach');
});
test('afterEach() async return value', async ({ page }) => {
await docsifyInit({
config: {
plugins: [
function (hook, vm) {
hook.afterEach((html, next) => {
setTimeout(() => {
next('<p>afterEach</p>');
}, 100);
});
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
test.afterEach(async ({ page }) => {
routeData = null;
});
await expect(page.locator('#main')).toContainText('afterEach');
test('success (200)', async ({ page }) => {
await docsifyInit({
config: {
plugins: [
function (hook, vm) {
hook.doneEach(html => {
console.log(JSON.stringify(vm.route));
});
},
],
},
});
expect(routeData).toHaveProperty('response');
expect(routeData.response).toHaveProperty('ok', true);
expect(routeData.response).toHaveProperty('status', 200);
expect(routeData.response).toHaveProperty('statusText', 'OK');
});
test('fail (404)', async ({ page }) => {
const link404Elm = page.locator('a[href*="404"]');
await docsifyInit({
markdown: {
sidebar: `
- [404](404.md)
`,
},
config: {
plugins: [
function (hook, vm) {
hook.doneEach(html => {
console.log(JSON.stringify(vm.route));
});
},
],
},
});
await link404Elm.click();
await waitForFunction(() => routeData?.response?.status === 404);
expect(routeData).toHaveProperty('response');
expect(routeData.response).toHaveProperty('ok', false);
expect(routeData.response).toHaveProperty('status', 404);
expect(routeData.response).toHaveProperty('statusText', 'Not Found');
});
});
});

View File

@ -221,7 +221,7 @@ test.describe('Virtual Routes - Generate Dynamic Content via Config', () => {
await navigateToRoute(page, '/d');
const mainElm = page.locator('#main');
await expect(mainElm).toContainText('404 - Not found');
await expect(mainElm).toContainText('404 - Not Found');
});
test('skip routes that returned a falsy value that is not a boolean', async ({
@ -263,7 +263,7 @@ test.describe('Virtual Routes - Generate Dynamic Content via Config', () => {
await navigateToRoute(page, '/multiple/matches');
const mainElm = page.locator('#main');
await expect(mainElm).toContainText('404 - Not found');
await expect(mainElm).toContainText('404 - Not Found');
});
test('skip routes that are not a valid string or function', async ({