Skip to content

Commit

Permalink
Update install report location to ext directory
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tombruijn committed Jan 19, 2022
1 parent 854390c commit 675fa15
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 21 deletions.

This file was deleted.

6 changes: 6 additions & 0 deletions packages/nodejs/.changesets/update-install-report-path.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions packages/nodejs/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,4 @@ build/
/ext/appsignal.version
/ext/libappsignal.a
/ext/libappsignal.dylib
/ext/*.report
8 changes: 1 addition & 7 deletions packages/nodejs/scripts/extension/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
43 changes: 43 additions & 0 deletions packages/nodejs/src/__tests__/diagnose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
8 changes: 1 addition & 7 deletions packages/nodejs/src/diagnose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion test/integration/diagnose

0 comments on commit 675fa15

Please sign in to comment.