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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion src/core/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default function (vm) {
nativeEmoji: false,
noCompileLinks: [],
noEmoji: false,
notFoundPage: true,
notFoundPage: false,
plugins: [],
relativePath: false,
repo: '',
Expand Down
6 changes: 4 additions & 2 deletions src/core/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
};

Expand Down
12 changes: 6 additions & 6 deletions src/core/render/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 > *')
Expand Down Expand Up @@ -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 => {
Expand Down
1 change: 1 addition & 0 deletions src/core/router/history/hash.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export class HashHistory extends History {
path,
file: this.getFile(path, true),
query: parseQuery(query),
response: {},
};
}

Expand Down
1 change: 1 addition & 0 deletions src/core/router/history/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export class HTML5History extends History {
path,
file: this.getFile(path),
query: parseQuery(query),
response: {},
};
}
}
24 changes: 17 additions & 7 deletions src/core/util/ajax.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};

/**
Expand Down Expand Up @@ -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(
_ =>
Expand All @@ -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.');
Expand All @@ -74,7 +84,7 @@ export function get(url, hasBar = false, headers = {}) {
},
});

success(result.content, result.opt);
success(result.content, result.opt, getResponseStatus(event));
}
});
},
Expand Down
180 changes: 131 additions & 49 deletions test/e2e/configuration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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,
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));

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;

page.on('console', msg => (consoleMsg = msg.text()));
page.on('pageerror', err => (errorMsg = err.message));

expect(errorMsg).toBeUndefined();
expect(consoleMsg).toContain('Docsify plugin error');
await expect(mainElm).toContainText('Hello World');
await expect(mainElm).toContainText('beforeEach');
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,
});
});
});

test('false', async ({ page }) => {
await docsifyInit({
config: {
notFoundPage: false,
},
});

page.on('console', msg => (consoleMsg = msg.text()));
page.on('pageerror', err => (errorMsg = err.message));
await expect(page.locator('#main')).toContainText('500');
});

await docsifyInit({
config: {
catchPluginErrors: false,
plugins: [
function (hook, vm) {
hook.ready(function () {
fail();
});
test('true with non-404 error', async ({ page }) => {
await docsifyInit({
config: {
notFoundPage: true,
},
routes: {
'_404.md': '',
},
],
},
markdown: {
homepage: '# Hello World',
},
// _logHTML: true,
});

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);
});
});
});
Loading