From 98f11a0685f05676669ded7fa852536c59991e80 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Tue, 19 Oct 2021 09:29:48 +0200 Subject: [PATCH] Diagnose print parsing error on missing report (#454) When the diagnose command is run and the installation report is missing the diagnose command crashes with an error. ``` UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'download_url' of undefined ``` This is because when the report can't be read the `getInstallationReport` function would only return an empty object, rather than the expected object with a parsing error. (Expected in the sense that's how our other integrations handle this type of error and that's what the diagnose tool on the server side expects.) With this change the report should no longer crash and the sent report includes the error about the missing installation report. I didn't know how to resolve a TypeScript issue with the optional `raw` key on the parsing error object, so I created a ParsingError type to indicate that it's a optional key. I couldn't get it to work otherwise. Part of #448 --- .../report-parse-errors-in-diagnose-report.md | 5 + packages/nodejs/src/cli/diagnose.ts | 94 +++++++++++-------- packages/nodejs/src/diagnose.ts | 24 ++++- test/integration/diagnose | 2 +- 4 files changed, 79 insertions(+), 46 deletions(-) create mode 100644 packages/nodejs/.changesets/report-parse-errors-in-diagnose-report.md diff --git a/packages/nodejs/.changesets/report-parse-errors-in-diagnose-report.md b/packages/nodejs/.changesets/report-parse-errors-in-diagnose-report.md new file mode 100644 index 00000000..2a93231c --- /dev/null +++ b/packages/nodejs/.changesets/report-parse-errors-in-diagnose-report.md @@ -0,0 +1,5 @@ +--- +bump: "patch" +--- + +The diagnose report will report parsing errors on reading or parsing the installation report. Previously, a missing installation report file would crash the diagnose tool. diff --git a/packages/nodejs/src/cli/diagnose.ts b/packages/nodejs/src/cli/diagnose.ts index 59f29939..44c04c4c 100644 --- a/packages/nodejs/src/cli/diagnose.ts +++ b/packages/nodejs/src/cli/diagnose.ts @@ -32,50 +32,62 @@ export class Diagnose { this.print_newline() console.log(`Extension installation report`) - console.log(` Installation result`) const installReport = data["installation"] - console.log(` Status: ${installReport["result"]["status"]}`) - const resultMessage = data["installation"]["result"]["message"] - if (resultMessage) { - console.log(` Message: ${resultMessage}`) - } - const resultError = data["installation"]["result"]["error"] - if (resultError) { - console.log(` Error: ${resultError}`) + const parsingError = installReport["parsing_error"] + if (parsingError) { + console.log(` Error found while parsing the report.`) + console.log(` Error: ${parsingError["error"]}`) + if (parsingError["raw"]) { + console.log(` Raw report:`) + console.log(parsingError["raw"]) + } + } else { + console.log(` Installation result`) + console.log(` Status: ${installReport["result"]["status"]}`) + const resultMessage = data["installation"]["result"]["message"] + if (resultMessage) { + console.log(` Message: ${resultMessage}`) + } + const resultError = data["installation"]["result"]["error"] + if (resultError) { + console.log(` Error: ${resultError}`) + } + + console.log(` Language details`) + console.log( + ` Node.js version: ${data["installation"]["language"]["version"]}` + ) + console.log(` Download details`) + console.log( + ` Download URL: ${data["installation"]["download"]["download_url"]}` + ) + console.log( + ` Checksum: ${data["installation"]["download"]["checksum"]}` + ) + console.log(` Build details`) + console.log(` Install time: ${data["installation"]["build"]["time"]}`) + console.log( + ` Architecture: ${data["installation"]["build"]["architecture"]}` + ) + console.log(` Target: ${data["installation"]["build"]["target"]}`) + console.log( + ` Musl override: ${data["installation"]["build"]["musl_override"]}` + ) + console.log( + ` Linux ARM override: ${data["installation"]["build"]["linux_arm_override"]}` + ) + console.log( + ` Library type: ${data["installation"]["build"]["library_type"]}` + ) + console.log(` Host details`) + console.log(` Root user: ${data["installation"]["host"]["root_user"]}`) + console.log( + ` Dependencies: ${this.format_value( + data["installation"]["host"]["dependencies"] + )}` + ) } - console.log(` Language details`) - console.log( - ` Node.js version: ${data["installation"]["language"]["version"]}` - ) - console.log(` Download details`) - console.log( - ` Download URL: ${data["installation"]["download"]["download_url"]}` - ) - console.log(` Checksum: ${data["installation"]["download"]["checksum"]}`) - console.log(` Build details`) - console.log(` Install time: ${data["installation"]["build"]["time"]}`) - console.log( - ` Architecture: ${data["installation"]["build"]["architecture"]}` - ) - console.log(` Target: ${data["installation"]["build"]["target"]}`) - console.log( - ` Musl override: ${data["installation"]["build"]["musl_override"]}` - ) - console.log( - ` Linux ARM override: ${data["installation"]["build"]["linux_arm_override"]}` - ) - console.log( - ` Library type: ${data["installation"]["build"]["library_type"]}` - ) - console.log(` Host details`) - console.log(` Root user: ${data["installation"]["host"]["root_user"]}`) - console.log( - ` Dependencies: ${this.format_value( - data["installation"]["host"]["dependencies"] - )}` - ) - this.print_newline() console.log(`Host information`) diff --git a/packages/nodejs/src/diagnose.ts b/packages/nodejs/src/diagnose.ts index 522b0557..faaed1c1 100644 --- a/packages/nodejs/src/diagnose.ts +++ b/packages/nodejs/src/diagnose.ts @@ -22,6 +22,12 @@ interface FileMetadata { writable?: boolean } +interface ParsingError { + error: string + backtrace: [] + raw?: string +} + export class DiagnoseTool { #config: Configuration #extension: Extension @@ -84,11 +90,21 @@ export class DiagnoseTool { } private getInstallationReport() { + let rawReport try { - const rawReport = fs.readFileSync(reportPath(), "utf8") - return rawReport ? JSON.parse(rawReport) : {} - } catch (e) { - return {} + rawReport = fs.readFileSync(reportPath(), "utf8") + return JSON.parse(rawReport) + } catch (error) { + const report = { + parsing_error: { + error: `${error.name}: ${error.message}`, + backtrace: error.stack.split("\n") + } as ParsingError + } + if (rawReport) { + report.parsing_error.raw = rawReport + } + return report } } diff --git a/test/integration/diagnose b/test/integration/diagnose index a2f513fd..a39f97a0 160000 --- a/test/integration/diagnose +++ b/test/integration/diagnose @@ -1 +1 @@ -Subproject commit a2f513fd7e158a6e194a7e90a9636f065963cfce +Subproject commit a39f97a08a8f7a87682a1c6f724156257caa5f3b