From d7b52e772d7d29ffafdc77d17ac5b83704dd65e4 Mon Sep 17 00:00:00 2001 From: Peter Bengtsson Date: Tue, 7 Feb 2023 12:22:44 -0500 Subject: [PATCH] catch typos near 'AUTOTITLE' (#34493) Co-authored-by: Sarah Schneider --- .../plugins/rewrite-local-links.js | 18 ++++++++++++++-- middleware/handle-errors.js | 4 +++- package-lock.json | 14 +++++++++++++ package.json | 1 + tests/README.md | 18 ++++++++++++++-- .../fixtures/content/get-started/foo/index.md | 1 + .../get-started/foo/typo-autotitling.md | 21 +++++++++++++++++++ tests/rendering-fixtures/internal-links.js | 17 ++++++++++++++- 8 files changed, 88 insertions(+), 6 deletions(-) create mode 100644 tests/fixtures/content/get-started/foo/typo-autotitling.md diff --git a/lib/render-content/plugins/rewrite-local-links.js b/lib/render-content/plugins/rewrite-local-links.js index 4e23e57467..9e3e635801 100644 --- a/lib/render-content/plugins/rewrite-local-links.js +++ b/lib/render-content/plugins/rewrite-local-links.js @@ -1,5 +1,6 @@ import path from 'path' import { visit } from 'unist-util-visit' +import { distance } from 'fastest-levenshtein' import { getPathWithoutLanguage, getVersionStringFromPath } from '../../path-utils.js' import { getNewVersionedPath } from '../../old-versions-utils.js' import patterns from '../../patterns.js' @@ -40,8 +41,21 @@ export default function rewriteLocalLinks(context) { node.properties.href = newHref } for (const child of node.children) { - if (child.value && AUTOTITLE.test(child.value)) { - promises.push(getNewTitleSetter(child, node.properties.href, context)) + if (child.value) { + if (AUTOTITLE.test(child.value)) { + promises.push(getNewTitleSetter(child, node.properties.href, context)) + } else if (process.env.NODE_ENV !== 'production') { + // Throw if the link text *almost* is AUTOTITLE + if ( + child.value.toUpperCase() === 'AUTOTITLE' || + distance(child.value.toUpperCase(), 'AUTOTITLE') <= 2 + ) { + throw new Error( + `Found link text '${child.value}', expected 'AUTOTITLE'. ` + + `Find the mention of the link text '${child.value}' and change it to 'AUTOTITLE'. Case matters.` + ) + } + } } } }) diff --git a/middleware/handle-errors.js b/middleware/handle-errors.js index 7db2c7ff42..a2270296e8 100644 --- a/middleware/handle-errors.js +++ b/middleware/handle-errors.js @@ -3,6 +3,8 @@ import { nextApp } from './next.js' import { setFastlySurrogateKey, SURROGATE_ENUMS } from './set-fastly-surrogate-key.js' import { errorCacheControl } from './cache-control.js' +const DEBUG_MIDDLEWARE_TESTS = Boolean(JSON.parse(process.env.DEBUG_MIDDLEWARE_TESTS || 'false')) + function shouldLogException(error) { const IGNORED_ERRORS = [ // Client connected aborted @@ -43,7 +45,7 @@ export default async function handleError(error, req, res, next) { // loading all the middlewares. setFastlySurrogateKey(res, SURROGATE_ENUMS.DEFAULT) } - } else if (process.env.NODE_ENV === 'test') { + } else if (DEBUG_MIDDLEWARE_TESTS) { console.warn('An error occurrred in some middleware handler', error) } diff --git a/package-lock.json b/package-lock.json index 3d72df984b..5fd369d2f1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28,6 +28,7 @@ "dotenv": "^16.0.1", "express": "^4.18.1", "express-timeout-handler": "^2.2.2", + "fastest-levenshtein": "1.0.16", "file-type": "18.2.0", "flat": "^5.0.2", "github-slugger": "^2.0.0", @@ -9343,6 +9344,14 @@ "url": "https://paypal.me/naturalintelligence" } }, + "node_modules/fastest-levenshtein": { + "version": "1.0.16", + "resolved": "https://registry.npmjs.org/fastest-levenshtein/-/fastest-levenshtein-1.0.16.tgz", + "integrity": "sha512-eRnCtTTtGZFpQCwhJiUOuxPQWRXVKYDn0b2PeHfXL6/Zi53SLAzAHfVhVWK2AryC/WH05kGfxhFIPvTF0SXQzg==", + "engines": { + "node": ">= 4.9.1" + } + }, "node_modules/fastq": { "version": "1.13.0", "resolved": "https://registry.npmjs.org/fastq/-/fastq-1.13.0.tgz", @@ -27613,6 +27622,11 @@ "strnum": "^1.0.4" } }, + "fastest-levenshtein": { + "version": "1.0.16", + "resolved": "https://registry.npmjs.org/fastest-levenshtein/-/fastest-levenshtein-1.0.16.tgz", + "integrity": "sha512-eRnCtTTtGZFpQCwhJiUOuxPQWRXVKYDn0b2PeHfXL6/Zi53SLAzAHfVhVWK2AryC/WH05kGfxhFIPvTF0SXQzg==" + }, "fastq": { "version": "1.13.0", "resolved": "https://registry.npmjs.org/fastq/-/fastq-1.13.0.tgz", diff --git a/package.json b/package.json index b05c78c0fd..b3546dee1d 100644 --- a/package.json +++ b/package.json @@ -30,6 +30,7 @@ "dotenv": "^16.0.1", "express": "^4.18.1", "express-timeout-handler": "^2.2.2", + "fastest-levenshtein": "1.0.16", "file-type": "18.2.0", "flat": "^5.0.2", "github-slugger": "^2.0.0", diff --git a/tests/README.md b/tests/README.md index 980e5ac734..6072ddcd38 100644 --- a/tests/README.md +++ b/tests/README.md @@ -6,7 +6,7 @@ but it's helpful to run tests locally before pushing your changes to GitHub. Tests are written using [jest](https://ghub.io/jest), a framework maintained -by Facebook and used by many teams at GitHub. +by Facebook and used by many teams at GitHub. Jest provides everything: a test runner, an assertion library, code coverage analysis, custom reporters for different types of test output, etc. @@ -98,7 +98,21 @@ In another terminal, type: START_JEST_SERVER=false jest tests/rendering/foo/bar.js ``` -Or whatever the testing command you use is. +Or whatever the testing command you use is. The `START_JEST_SERVER` environment variable needs to be set to `false`, or else `jest` will try to start a server on `:4000` too. + +### Debugging middleware errors + +By default, errors handled by the middleware are dealt with just like +any error in production. It's common to have end-to-end tests that expect +a page to throw a 500 Internal Server Error response. + +If you don't expect that and you might struggle to see exactly where the +error is happening, set `$DEBUG_MIDDLEWARE_TESTS` to `true`. For example: + +```sh +export DEBUG_MIDDLEWARE_TESTS=true +jest tests/rendering/ -b +``` diff --git a/tests/fixtures/content/get-started/foo/index.md b/tests/fixtures/content/get-started/foo/index.md index df893b02b6..c1879b3b87 100644 --- a/tests/fixtures/content/get-started/foo/index.md +++ b/tests/fixtures/content/get-started/foo/index.md @@ -10,5 +10,6 @@ versions: children: - /bar - /autotitling + - /typo-autotitling - /cross-version-linking --- diff --git a/tests/fixtures/content/get-started/foo/typo-autotitling.md b/tests/fixtures/content/get-started/foo/typo-autotitling.md new file mode 100644 index 0000000000..bc04908c45 --- /dev/null +++ b/tests/fixtures/content/get-started/foo/typo-autotitling.md @@ -0,0 +1,21 @@ +--- +title: Typo autotitling +intro: If the author typos the word "AUTOTITLE" it will throw +versions: + fpt: '*' + ghes: '*' + ghae: '*' +type: how_to +--- + +## Example + +{% ifversion ghes %} + +"[Autotitle](/get-started/quickstart/hello-world)." + +{% else %} + +"[AUTOTITLES](/get-started/quickstart/hello-world)." + +{% endif %} diff --git a/tests/rendering-fixtures/internal-links.js b/tests/rendering-fixtures/internal-links.js index ea4f4ecfcb..7002c6b915 100644 --- a/tests/rendering-fixtures/internal-links.js +++ b/tests/rendering-fixtures/internal-links.js @@ -1,4 +1,4 @@ -import { getDOM } from '../helpers/e2etest.js' +import { get, getDOM } from '../helpers/e2etest.js' import enterpriseServerReleases from '../../lib/enterprise-server-releases.js' import { allVersions } from '../../lib/all-versions.js' @@ -14,6 +14,21 @@ describe('autotitle', () => { // There are 4 links on the `autotitling.md` content. expect.assertions(4) }) + + test('typos lead to error when NODE_ENV !== production', async () => { + // The fixture typo-autotitling.md contains two different typos + // of the word "AUTOTITLE", separated by `{% if version ghes %}` + { + const res = await get('/get-started/foo/typo-autotitling', { followRedirects: true }) + expect(res.statusCode).toBe(500) + } + { + const res = await get('/enterprise-server@latest/get-started/foo/typo-autotitling', { + followRedirects: true, + }) + expect(res.statusCode).toBe(500) + } + }) }) describe('cross-version-links', () => {