From 675fa1510dcc162adf87488fff07f237e2d3c8f0 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Tue, 18 Jan 2022 11:33:20 +0100 Subject: [PATCH] Update install report location to ext directory In PR #545 we've merged the nodejs-ext package into the nodejs package. It no longer fails the package install on extension installation failure, which means the ext directory remains after a installation failure. We can now store the install report file in the package's `ext` directory, like we do in all the other integrations. This means we no longer need to generate a filename for the install report to store it in the system tmp directory. Closes #550 --- ...fix-install-report-path-name-generation.md | 6 --- .../.changesets/update-install-report-path.md | 6 +++ packages/nodejs/.gitignore | 1 + packages/nodejs/scripts/extension/report.js | 8 +--- .../nodejs/src/__tests__/diagnose.test.ts | 43 +++++++++++++++++++ packages/nodejs/src/diagnose.ts | 8 +--- test/integration/diagnose | 2 +- 7 files changed, 53 insertions(+), 21 deletions(-) delete mode 100644 packages/nodejs/.changesets/fix-install-report-path-name-generation.md create mode 100644 packages/nodejs/.changesets/update-install-report-path.md diff --git a/packages/nodejs/.changesets/fix-install-report-path-name-generation.md b/packages/nodejs/.changesets/fix-install-report-path-name-generation.md deleted file mode 100644 index 5306aa26..00000000 --- a/packages/nodejs/.changesets/fix-install-report-path-name-generation.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -bump: "patch" -type: "fix" ---- - -Fix install report path name generation. This fixes the diagnose report not being able to find the path where the install report is stored. diff --git a/packages/nodejs/.changesets/update-install-report-path.md b/packages/nodejs/.changesets/update-install-report-path.md new file mode 100644 index 00000000..815d6502 --- /dev/null +++ b/packages/nodejs/.changesets/update-install-report-path.md @@ -0,0 +1,6 @@ +--- +bump: "patch" +type: "change" +--- + +Update install report path location. The "nodejs" package now stores the install report in its own package path, with the rest of the extension files. This makes it easier to access by the diagnose report. diff --git a/packages/nodejs/.gitignore b/packages/nodejs/.gitignore index d080f84a..93c969b2 100644 --- a/packages/nodejs/.gitignore +++ b/packages/nodejs/.gitignore @@ -79,3 +79,4 @@ build/ /ext/appsignal.version /ext/libappsignal.a /ext/libappsignal.dylib +/ext/*.report diff --git a/packages/nodejs/scripts/extension/report.js b/packages/nodejs/scripts/extension/report.js index 1e2cd7f4..5054f9d9 100644 --- a/packages/nodejs/scripts/extension/report.js +++ b/packages/nodejs/scripts/extension/report.js @@ -80,13 +80,7 @@ function createDownloadReport(report) { // This implementation should match the `packages/nodejs/src/diagnose.ts` // implementation to generate the same path. function reportPath() { - // Navigate up to the app dir. Move up the scripts dir, package dir, - // @appsignal dir and node_modules dir. - const appPath = path.join(__dirname, "../../../../../") - const hash = crypto.createHash("sha256") - hash.update(appPath) - const reportPathDigest = hash.digest("hex") - return path.join(`/tmp/appsignal-${reportPathDigest}-install.report`) + return path.join(__dirname, "../../ext/install.report") } module.exports = { diff --git a/packages/nodejs/src/__tests__/diagnose.test.ts b/packages/nodejs/src/__tests__/diagnose.test.ts index 3d39d020..a43a29a7 100644 --- a/packages/nodejs/src/__tests__/diagnose.test.ts +++ b/packages/nodejs/src/__tests__/diagnose.test.ts @@ -42,6 +42,49 @@ describe("DiagnoseTool", () => { expect(install).toHaveProperty("language") expect(install).toHaveProperty("result") }) + + it("returns an error report on failure to read the install report", async () => { + const fsReadSpy = jest + .spyOn(fs, "readFileSync") + .mockImplementation(() => { + throw new Error("uh oh") + }) + const output = await tool.generate() + + const install = output.installation + expect(install).toMatchObject({ + parsing_error: { + error: expect.any(String), + backtrace: expect.any(Array) + } + }) + expect(install).not.toHaveProperty("build") + expect(install).not.toHaveProperty("download") + expect(install).not.toHaveProperty("host") + expect(install).not.toHaveProperty("language") + expect(install).not.toHaveProperty("result") + }) + + it("returns an error report on failure to parse the install report", async () => { + const fsReadSpy = jest + .spyOn(fs, "readFileSync") + .mockImplementation(() => "not JSON") + const output = await tool.generate() + + const install = output.installation + expect(install).toMatchObject({ + parsing_error: { + error: expect.any(String), + backtrace: expect.any(Array), + raw: "not JSON" + } + }) + expect(install).not.toHaveProperty("build") + expect(install).not.toHaveProperty("download") + expect(install).not.toHaveProperty("host") + expect(install).not.toHaveProperty("language") + expect(install).not.toHaveProperty("result") + }) }) it("returns the log_dir_path", async () => { diff --git a/packages/nodejs/src/diagnose.ts b/packages/nodejs/src/diagnose.ts index 149839ce..266c350b 100644 --- a/packages/nodejs/src/diagnose.ts +++ b/packages/nodejs/src/diagnose.ts @@ -268,13 +268,7 @@ export class DiagnoseTool { // This implementation should match the `scripts/extension/report.js` // implementation to generate the same path. function reportPath(): string { - // Navigate up to the app dir. Move up the src dir, package dir, @appsignal - // dir and node_modules dir. - const appPath = path.join(__dirname, "../../../../") - const hash = createHash("sha256") - hash.update(appPath) - const reportPathDigest = hash.digest("hex") - return path.join(`/tmp/appsignal-${reportPathDigest}-install.report`) + return path.join(__dirname, "../ext/install.report") } function getPathType(stats: fs.Stats) { diff --git a/test/integration/diagnose b/test/integration/diagnose index a57bfade..ad7f5062 160000 --- a/test/integration/diagnose +++ b/test/integration/diagnose @@ -1 +1 @@ -Subproject commit a57bfadeaf0f7f8434a3920f54de5d84a4518b34 +Subproject commit ad7f5062b111ed3586b94a768e0642eee9d4a9f2