diff --git a/src/main.ts b/src/main.ts index fa79beb..013e200 100644 --- a/src/main.ts +++ b/src/main.ts @@ -9,6 +9,7 @@ import fs = require("fs"); import path = require("path"); import mdEscape = require("markdown-escape"); import randomSeed = require("random-seed"); +import { getErrorMessageFromStack, getHash, getHashForStack } from "./utils/hashStackTrace"; interface Params { /** @@ -84,9 +85,31 @@ export type RepoStatus = | "Detected no interesting changes" ; +interface TSServerResult { + oldServerFailed: boolean; + oldSpawnResult?: SpawnResult; + newServerFailed: boolean; + newSpawnResult: SpawnResult; + replayScriptPath: string; + installCommands: ip.InstallCommand[]; +} + +interface Summary { + tsServerResult: TSServerResult; + repo: git.Repo; + oldTsEntrypointPath: string; + rawErrorArtifactPath: string; + replayScriptPath: string; + repoDir: string; + downloadDir: string; + replayScriptArtifactPath: string; + replayScriptName: string; +} + interface RepoResult { readonly status: RepoStatus; readonly summary?: string; + readonly tsServerResult?: TSServerResult; readonly replayScriptPath?: string; readonly rawErrorPath?: string; } @@ -203,8 +226,6 @@ async function getTsServerRepoResult( ? (await installPackagesAndGetCommands(repo, downloadDir, repoDir, monorepoPackages, /*cleanOnFailure*/ true, diagnosticOutput))! : []; - const isUserTestRepo = !repo.url; - const replayScriptName = path.basename(replayScriptArtifactPath); const replayScriptPath = path.join(downloadDir, replayScriptName); @@ -302,110 +323,173 @@ async function getTsServerRepoResult( } } - const owner = repo.owner ? `${mdEscape(repo.owner)}/` : ""; - const url = repo.url ? `(${repo.url})` : ""; + const tsServerResult = { + oldServerFailed, + oldSpawnResult, + newServerFailed, + newSpawnResult, + replayScriptPath, + installCommands, + }; + + if (oldServerFailed && !newServerFailed) { + return { status: "Detected interesting changes", tsServerResult } + } + if (!newServerFailed) { + return { status: "Detected no interesting changes" }; + } - let summary = `## [${owner}${mdEscape(repo.name)}]${url}\n`; + return { status: "Detected interesting changes", tsServerResult, replayScriptPath, rawErrorPath }; + } + catch (err) { + reportError(err, `Error running tsserver on ${repo.url ?? repo.name}`); + return { status: "Unknown failure" }; + } + finally { + console.log(`Done ${repo.url ?? repo.name}`); + logStepTime(diagnosticOutput, repo, "language service", lsStart); + } +} - if (oldServerFailed) { - const oldServerError = oldSpawnResult?.stdout - ? prettyPrintServerHarnessOutput(oldSpawnResult.stdout, /*filter*/ true) +function groupErrors(summaries: Summary[]) { + const groupedOldErrors = new Map(); + const groupedNewErrors = new Map(); + let group: Map; + let error: ServerHarnessOutput | string; + for (const summary of summaries) { + if (summary.tsServerResult.newServerFailed) { + // Group new errors + error = parseServerHarnessOutput(summary.tsServerResult.newSpawnResult!.stdout); + group = groupedNewErrors; + } + else if (summary.tsServerResult.oldServerFailed) { + // Group old errors + const { oldSpawnResult } = summary.tsServerResult; + error = oldSpawnResult?.stdout + ? parseServerHarnessOutput(oldSpawnResult.stdout) : `Timed out after ${executionTimeout} ms`; - summary += ` + + group = groupedOldErrors; + } + else { + continue; + } + + const key = typeof error === "string" ? getHash([error]) : getHashForStack(error.message); + const value = group.get(key) ?? []; + value.push(summary); + group.set(key, value); + } + + return { groupedOldErrors, groupedNewErrors } +} + +function getErrorMessage(output: string): string { + const error = parseServerHarnessOutput(output); + + return typeof error === "string" ? error : getErrorMessageFromStack(error.message); +} + +function createOldErrorSummary(summaries: Summary[]): string { + const { oldSpawnResult } = summaries[0].tsServerResult; + + const oldServerError = oldSpawnResult?.stdout + ? prettyPrintServerHarnessOutput(oldSpawnResult.stdout, /*filter*/ true) + : `Timed out after ${executionTimeout} ms`; + + const errorMessage = oldSpawnResult?.stdout ? getErrorMessage(oldSpawnResult.stdout) : oldServerError; + + let text = `
-:warning: Note that ${path.basename(path.dirname(path.dirname(oldTsServerPath)))} had errors :warning: +${errorMessage} \`\`\` ${oldServerError} \`\`\` -
- +

Repos no longer reporting the error

+
    `; - if (!newServerFailed) { - summary += ` -:tada: New server no longer has errors :tada: + + for (const summary of summaries) { + const owner = summary.repo.owner ? `${mdEscape(summary.repo.owner)}/` : ""; + const url = summary.repo.url ?? ""; + + text += `
  • ${owner + mdEscape(summary.repo.name)}
    • \n` + } + + text += ` +
+ `; - return { status: "Detected interesting changes", summary } - } - } - - if (!newServerFailed) { - return { status: "Detected no interesting changes" }; - } - summary += ` + return text; +} + +async function createNewErrorSummaryAsync(summaries: Summary[]): Promise { + let text = `

${getErrorMessage(summaries[0].tsServerResult.newSpawnResult.stdout)}

\`\`\` -${prettyPrintServerHarnessOutput(newSpawnResult.stdout, /*filter*/ true)} +${prettyPrintServerHarnessOutput(summaries[0].tsServerResult.newSpawnResult.stdout, /*filter*/ true)} \`\`\` -That is a filtered view of the text. To see the raw error text, go to ${rawErrorArtifactPath} in the artifact folder\n -`; - summary += ` +

Affected repos

`; + + for (const summary of summaries) { + const owner = summary.repo.owner ? `${mdEscape(summary.repo.owner)}/` : ""; + const url = summary.repo.url ?? ""; + + text += `
-

Last few requests

 +${owner + mdEscape(summary.repo.name)} +Raw error text: ${summary.rawErrorArtifactPath} in the artifact folder +

Last few requests

\`\`\`json -${fs.readFileSync(replayScriptPath, { encoding: "utf-8" }).split(/\r?\n/).slice(-5).join("\n")} +${fs.readFileSync(summary.replayScriptPath, { encoding: "utf-8" }).split(/\r?\n/).slice(-5).join("\n")} \`\`\` -
- -`; - - // Markdown doesn't seem to support a
list item, so this chunk is in HTML - - summary += `
-

Repro Steps

 +

Repro steps

    `; - if (isUserTestRepo) { - summary += `
  1. Download user test ${repo.name}
    2. \n`; + // No url means is user test repo + if (!summary.repo.url) { + text += `
  2. Download user test ${summary.repo.name}
    3. \n`; } else { - summary += `
  3. git clone ${repo.url!} --recurse-submodules
    4. \n`; + text += `
  4. git clone ${summary.repo.url} --recurse-submodules
    5. \n`; try { console.log("Extracting commit SHA for repro steps"); - const commit = (await execAsync(repoDir, `git rev-parse @`)).trim(); - summary += `
  5. In dir ${repo.name}, run git reset --hard ${commit}
    6. \n`; + const commit = (await execAsync(summary.repoDir, `git rev-parse @`)).trim(); + text += `
  6. In dir ${summary.repo.name}, run git reset --hard ${commit}
    7. \n`; } catch { } } - if (installCommands.length > 1) { - summary += "
  7. Install packages (exact steps are below, but it might be easier to follow the repo readme)
      \n"; + if (summary.tsServerResult.installCommands.length > 1) { + text += "
    1. Install packages (exact steps are below, but it might be easier to follow the repo readme)
        \n"; } - for (const command of installCommands) { - summary += `
      1. In dir ${path.relative(downloadDir, command.directory)}, run ${command.tool} ${command.arguments.join(" ")}
        2. \n`; + for (const command of summary.tsServerResult.installCommands) { + text += `
      2. In dir ${path.relative(summary.downloadDir, command.directory)}, run ${command.tool} ${command.arguments.join(" ")}
        3. \n`; } - if (installCommands.length > 1) { - summary += "
      \n"; + if (summary.tsServerResult.installCommands.length > 1) { + text += "
    \n"; } // The URL of the artifact can be determined via AzDO REST APIs, but not until after the artifact is published - summary += `
  8. Back in the initial folder, download ${replayScriptArtifactPath} from the artifact folder
    9. \n`; - summary += `
  9. npm install --no-save @typescript/server-replay
    10. \n`; - summary += `
  10. npx tsreplay ./${repo.name} ./${replayScriptName} path/to/tsserver.js
    11. \n`; - summary += `
  11. npx tsreplay --help to learn about helpful switches for debugging, logging, etc
    12. \n`; + text += `
  12. Back in the initial folder, download ${summary.replayScriptArtifactPath} from the artifact folder
    13. \n`; + text += `
  13. npm install --no-save @typescript/server-replay
    14. \n`; + text += `
  14. npx tsreplay ./${summary.repo.name} ./${summary.replayScriptName} path/to/tsserver.js
    15. \n`; + text += `
  15. npx tsreplay --help to learn about helpful switches for debugging, logging, etc
    16. \n`; - summary += `
+ text += `
- `; - - return { status: "Detected interesting changes", summary, replayScriptPath, rawErrorPath }; - } - catch (err) { - reportError(err, `Error running tsserver on ${repo.url ?? repo.name}`); - return { status: "Unknown failure" }; - } - finally { - console.log(`Done ${repo.url ?? repo.name}`); - logStepTime(diagnosticOutput, repo, "language service", lsStart); } + + return text; } // Exported for testing @@ -658,6 +742,8 @@ export async function mainAsync(params: GitParams | UserParams): Promise { const isPr = params.testType === "user" && !!params.prNumber + var summaries: Summary[] = []; + let i = 1; for (const repo of repos) { console.log(`Starting #${i++} / ${repos.length}: ${repo.url ?? repo.name}`); @@ -672,16 +758,36 @@ export async function mainAsync(params: GitParams | UserParams): Promise { : repo.name; const replayScriptFileName = `${repoPrefix}.${replayScriptFileNameSuffix}`; const rawErrorFileName = `${repoPrefix}.${rawErrorFileNameSuffix}`; - const { status, summary, replayScriptPath, rawErrorPath } = params.entrypoint === "tsc" + + const rawErrorArtifactPath = path.join(params.resultDirName, rawErrorFileName); + const replayScriptArtifactPath = path.join(params.resultDirName, replayScriptFileName); + + const { status, summary, tsServerResult, replayScriptPath, rawErrorPath } = params.entrypoint === "tsc" ? await getTscRepoResult(repo, userTestsDir, oldTsEntrypointPath, newTsEntrypointPath, params.buildWithNewWhenOldFails, downloadDir, diagnosticOutput) - : await getTsServerRepoResult(repo, userTestsDir, oldTsEntrypointPath, newTsEntrypointPath, downloadDir, path.join(params.resultDirName, replayScriptFileName), path.join(params.resultDirName, rawErrorFileName), diagnosticOutput, isPr); + : await getTsServerRepoResult(repo, userTestsDir, oldTsEntrypointPath, newTsEntrypointPath, downloadDir, replayScriptArtifactPath, rawErrorArtifactPath, diagnosticOutput, isPr); console.log(`Repo ${repo.url ?? repo.name} had status "${status}"`); statusCounts[status] = (statusCounts[status] ?? 0) + 1; - if (summary) { + if (summary) { const resultFileName = `${repoPrefix}.${resultFileNameSuffix}`; await fs.promises.writeFile(path.join(resultDirPath, resultFileName), summary, { encoding: "utf-8" }); + } + + if (tsServerResult) { + summaries.push({ + tsServerResult, + repo, + oldTsEntrypointPath, + rawErrorArtifactPath, + replayScriptPath: path.join(downloadDir, path.basename(replayScriptArtifactPath)), + repoDir: path.join(downloadDir, repo.name), + downloadDir, + replayScriptArtifactPath, + replayScriptName: path.basename(replayScriptArtifactPath), + }); + } + if (summary || tsServerResult) { // In practice, there will only be a replay script when the entrypoint is tsserver // There can be replay steps without a summary, but then they're not interesting if (replayScriptPath) { @@ -690,9 +796,7 @@ export async function mainAsync(params: GitParams | UserParams): Promise { if (rawErrorPath) { await fs.promises.copyFile(rawErrorPath, path.join(resultDirPath, rawErrorFileName)); } - } - } finally { // Throw away the repo so we don't run out of space @@ -727,6 +831,25 @@ export async function mainAsync(params: GitParams | UserParams): Promise { } } + // Group errors and create summary files. + if (summaries.length > 0) { + const { groupedOldErrors, groupedNewErrors } = groupErrors(summaries); + + for (let [key, value] of groupedOldErrors) { + const summary = createOldErrorSummary(value); + const resultFileName = `!${key}.${resultFileNameSuffix}`; // Exclamation point makes the file to be put first when ordering. + + await fs.promises.writeFile(path.join(resultDirPath, resultFileName), summary, { encoding: "utf-8" }); + } + + for (let [key, value] of groupedNewErrors) { + const summary = await createNewErrorSummaryAsync(value); + const resultFileName = `${key}.${resultFileNameSuffix}`; + + await fs.promises.writeFile(path.join(resultDirPath, resultFileName), summary, { encoding: "utf-8" }); + } + } + if (params.tmpfs) { await execAsync(processCwd, "sudo rm -rf " + downloadDir); await execAsync(processCwd, "sudo rm -rf " + oldTscDirPath); @@ -993,4 +1116,4 @@ async function downloadTsNpmAsync(cwd: string, version: string, entrypoint: TsEn } return { tsEntrypointPath, resolvedVersion }; -} +} \ No newline at end of file diff --git a/src/postGithubComments.ts b/src/postGithubComments.ts index 8933b8b..9fddbaf 100644 --- a/src/postGithubComments.ts +++ b/src/postGithubComments.ts @@ -55,6 +55,9 @@ else { summary = `Everything looks good!`; } +// Files starting with an exclamation point are old server errors. +const hasOldErrors = pu.glob(resultDirPath, `**/!*.${resultFileNameSuffix}`).length !== 0; + const resultPaths = pu.glob(resultDirPath, `**/*.${resultFileNameSuffix}`).sort((a, b) => path.basename(a).localeCompare(path.basename(b))); const outputs = resultPaths.map(p => fs.readFileSync(p, { encoding: "utf-8" }).replace(new RegExp(artifactFolderUrlPlaceholder, "g"), artifactsUri)); @@ -67,9 +70,10 @@ if (!outputs.length) { git.createComment(+prNumber, +commentNumber, postResult, [header]); } else { + const oldErrorHeader = `

:warning: Old server errors :warning:

`; const openDetails = `\n\n
\nDetails\n\n`; const closeDetails = `\n
`; - const initialHeader = header + openDetails; + const initialHeader = header + openDetails + (hasOldErrors ? oldErrorHeader : ''); const continuationHeader = `@${userToTag} Here are some more interesting changes from running the ${suiteDescription} suite${openDetails}`; const trunctationSuffix = `\n:error: Truncated - see log for full output :error:`; diff --git a/src/utils/hashStackTrace.ts b/src/utils/hashStackTrace.ts new file mode 100644 index 0000000..3246d69 --- /dev/null +++ b/src/utils/hashStackTrace.ts @@ -0,0 +1,18 @@ + import { createHash } from "crypto"; + +export function getHash(methods: string[]): string { + const lines = methods.join("\n"); + return createHash("md5").update(lines).digest("hex"); +} + +export function getHashForStack(stack: string): string { + const stackLines = stack.split(/\r?\n/); + + return getHash(stackLines); +} + +export function getErrorMessageFromStack(stack: string): string { + const stackLines = stack.split(/\r?\n/, 2); + + return stackLines[1]; +} \ No newline at end of file