From c7370e5ee0cf0b8e621026b6f1e47df1a461baae Mon Sep 17 00:00:00 2001 From: Peter Bengtsson Date: Mon, 18 Dec 2023 16:31:47 -0500 Subject: [PATCH] Post PR annotations on failing AUTOTITLE errors (#45129) Co-authored-by: Rachael Sewell --- package-lock.json | 19 ++-- package.json | 1 + .../unified/rewrite-local-links.js | 86 +++++++++++++++++-- 3 files changed, 91 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index a158cc4b0a..50c628ccc8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -85,6 +85,7 @@ "semver": "^7.5.4", "sharp": "0.32.6", "slash": "^5.0.0", + "strip-ansi": "7.1.0", "strip-html-comments": "^1.0.0", "styled-components": "^5.3.5", "swr": "^2.2.2", @@ -2736,9 +2737,9 @@ } }, "node_modules/@primer/react/node_modules/@oddbird/popover-polyfill": { - "version": "0.3.5", - "resolved": "https://registry.npmjs.org/@oddbird/popover-polyfill/-/popover-polyfill-0.3.5.tgz", - "integrity": "sha512-NpbjRxlc99/w/fWhOLMYl0m314HTiEwBtWfAs3z65rEw8pCX+NgUomscG3Ao6bbxaT3tLtSXRkDLx3u6gazIBw==" + "version": "0.3.7", + "resolved": "https://registry.npmjs.org/@oddbird/popover-polyfill/-/popover-polyfill-0.3.7.tgz", + "integrity": "sha512-WNthEIPPXnFQkumLby6yVxhyOcA/GtMnlByHwEglMO9WZckoaqidnpLp2JFzAh2RDOZxn+Xt3ffSMKId9cPjOQ==" }, "node_modules/@primer/view-components": { "version": "0.1.4", @@ -3151,9 +3152,9 @@ "license": "MIT" }, "node_modules/@types/semver": { - "version": "7.5.5", - "resolved": "https://registry.npmjs.org/@types/semver/-/semver-7.5.5.tgz", - "integrity": "sha512-+d+WYC1BxJ6yVOgUgzK8gWvp5qF8ssV5r4nsDcZWKRWcDQLQ619tvWAxJQYGgBrO1MnLJC7a5GtiYsAoQ47dJg==", + "version": "7.5.6", + "resolved": "https://registry.npmjs.org/@types/semver/-/semver-7.5.6.tgz", + "integrity": "sha512-dn1l8LaMea/IjDoHNd9J52uBbInB796CDffS6VdIxvqYCPSG0V0DzHp76GpaWnlhg88uYyPbXCDIowa86ybd5A==", "dev": true }, "node_modules/@types/stack-utils": { @@ -14647,9 +14648,9 @@ } }, "node_modules/streamx": { - "version": "2.15.5", - "resolved": "https://registry.npmjs.org/streamx/-/streamx-2.15.5.tgz", - "integrity": "sha512-9thPGMkKC2GctCzyCUjME3yR03x2xNo0GPKGkRw2UMYN+gqWa9uqpyNWhmsNCutU5zHmkUum0LsCRQTXUgUCAg==", + "version": "2.15.6", + "resolved": "https://registry.npmjs.org/streamx/-/streamx-2.15.6.tgz", + "integrity": "sha512-q+vQL4AAz+FdfT137VF69Cc/APqUbxy+MDOImRrMvchJpigHj9GksgDU2LYbO9rx7RX6osWgxJB2WxhYv4SZAw==", "dependencies": { "fast-fifo": "^1.1.0", "queue-tick": "^1.0.1" diff --git a/package.json b/package.json index f52059695c..fc76f1a275 100644 --- a/package.json +++ b/package.json @@ -303,6 +303,7 @@ "semver": "^7.5.4", "sharp": "0.32.6", "slash": "^5.0.0", + "strip-ansi": "7.1.0", "strip-html-comments": "^1.0.0", "styled-components": "^5.3.5", "swr": "^2.2.2", diff --git a/src/content-render/unified/rewrite-local-links.js b/src/content-render/unified/rewrite-local-links.js index d22e2c7f09..f1482015a0 100644 --- a/src/content-render/unified/rewrite-local-links.js +++ b/src/content-render/unified/rewrite-local-links.js @@ -1,4 +1,7 @@ import path from 'path' +import fs from 'fs' + +import stripAnsi from 'strip-ansi' import { visit } from 'unist-util-visit' import { distance } from 'fastest-levenshtein' import { getPathWithoutLanguage, getVersionStringFromPath } from '#src/frame/lib/path-utils.js' @@ -13,9 +16,65 @@ import findPage from '#src/frame/lib/find-page.js' const isProd = process.env.NODE_ENV === 'production' +// This way, if you *set* the `LOG_ERROR_ANNOTATIONS` env var, whatever its +// value is, it determines it. But if it's not set, the default is to look +// for a truty value in `process.env.CI`. +const CI = Boolean(JSON.parse(process.env.CI || 'false')) +const LOG_ERROR_ANNOTATIONS = + CI || Boolean(JSON.parse(process.env.LOG_ERROR_ANNOTATIONS || 'false')) + const supportedPlans = new Set(Object.values(allVersions).map((v) => v.plan)) const externalRedirects = readJsonFile('./src/redirects/lib/external-sites.json') +// The reason we "memoize" which lines we've logged is because the same +// error might happen more than once in the whole space of one CI run. +const _logged = new Set() + +// Printing this to stdout in this format, will automatically be picked up +// by Actions to turn that into a PR inline annotation. +function logError(file, line, message, title = 'Error') { + if (LOG_ERROR_ANNOTATIONS) { + const hash = `${file}:${line}:${message}` + if (_logged.has(hash)) return + _logged.add(hash) + message = stripAnsi( + // copied from: https://github.com/actions/toolkit/blob/main/packages/core/src/command.ts + message.replace(/%/g, '%25').replace(/\r/g, '%0D').replace(/\n/g, '%0A'), + ) + const error = `::error file=${file},line=${line},title=${title}::${message}` + console.log(error) + } +} + +function getFrontmatterOffset(filePath) { + const rawContent = fs.readFileSync(filePath, 'utf-8') + let delimiters = 0 + let count = 0 + // The frontmatter is wedged between two `---` lines. But the content + // doesn't necessarily start after the second `---`. If the `.md` file looks + // like this: + // + // 1) --- + // 2) title: Foo + // 3) --- + // 4) + // 5) # Introduction + // 6) Bla bla + // + // Then line one of the *content* that is processed, starts at line 5. + // because after the matter and content is separated, the content portion + // is whitespace trimmed. + for (const line of rawContent.split(/\n/g)) { + count++ + if (line === '---') { + delimiters++ + } else if (delimiters === 2 && line) { + return count + } + } + return 0 +} + // Meaning it can be 'AUTOTITLE ' or ' AUTOTITLE' or 'AUTOTITLE' const AUTOTITLE = /^\s*AUTOTITLE\s*$/ @@ -57,12 +116,17 @@ export default function rewriteLocalLinks(context) { // in English, but all AUTOTITLE links in the current language. const newHref = getNewHref(node, autotitleLanguage || currentLanguage, currentVersion) if (newHref) { + node.properties._originalHref = node.properties.href node.properties.href = newHref } for (const child of node.children) { if (child.value) { if (AUTOTITLE.test(child.value)) { - nodes.push({ href: node.properties.href, child }) + nodes.push({ + href: node.properties.href, + child, + originalHref: node.properties._originalHref, + }) } else if ( // This means CI and local dev process.env.NODE_ENV !== 'production' && @@ -99,18 +163,28 @@ export default function rewriteLocalLinks(context) { }) } - await Promise.all(nodes.map(({ href, child }) => getNewTitleSetter(child, href, context))) + await Promise.all( + nodes.map(({ href, child, originalHref }) => + getNewTitleSetter(child, href, context, originalHref), + ), + ) } } -async function getNewTitleSetter(child, href, context) { - child.value = await getNewTitle(href, context) +async function getNewTitleSetter(child, href, context, originalHref) { + child.value = await getNewTitle(href, context, child, originalHref) } -async function getNewTitle(href, context) { +async function getNewTitle(href, context, child, originalHref) { const page = findPage(href, context.pages, context.redirects) if (!page) { - throw new TitleFromAutotitleError(`Unable to find Page by href '${href}'`) + const line = child.position.start.line + getFrontmatterOffset(context.page.fullPath) + const message = `Unable to find Page by '${originalHref || href}'. + To fix it, look at ${ + context.page.fullPath + } on line ${line} and see if the link is correct and active.` + logError(context.page.fullPath, line, message) + throw new TitleFromAutotitleError(message) } return await page.renderProp('title', context, { textOnly: true }) }