Skip to content

Commit

Permalink
Show extension load error on require
Browse files Browse the repository at this point in the history
Before this commit, any errors in loading the extension wrapper
module wouldn't be shown if the `Extension` object wasn't started,
and the `BaseClient` object would not attempt to start the
extension if the extension wrapper wasn't loaded, meaning that
errors in loading the extension wrapper were, in practice, never
shown.

This commit changes those errors to be emitted when the extension
wrapper module is required, instead of when the `Extension` object
is started.
  • Loading branch information
unflxw committed Feb 11, 2022
1 parent 0af54b7 commit 083a39a
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "fix"
---

Show errors in loading the extension when the `@appsignal/nodejs` module is required.
16 changes: 12 additions & 4 deletions packages/nodejs/src/__tests__/extension.failure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,26 @@ describe("Extension", () => {
ext.stop()
})

it("logs a warning when the module is required", () => {
const errorSpy = jest.spyOn(console, "error")

jest.resetModules()
require("../extension")

expect(errorSpy).toHaveBeenLastCalledWith(
"AppSignal extension not loaded. This could mean that your current " +
"environment isn't supported, or that another error has occurred."
)
})

it("is not loaded", () => {
expect(Extension.isLoaded).toEqual(false)
})

it("does not start the client", () => {
const warnSpy = jest.spyOn(console, "warn")
expect(() => {
ext.start()
}).not.toThrow()
expect(warnSpy).toHaveBeenLastCalledWith(
"AppSignal extension not loaded. This could mean that your current environment isn't supported, or that another error has occurred."
)
})

it("does not error on stopping the client", () => {
Expand Down
14 changes: 1 addition & 13 deletions packages/nodejs/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,7 @@ export class Extension {
* Starts the extension.
*/
public start() {
try {
extension.start()
} catch (e) {
if (e.message === "Extension module not loaded") {
console.warn(
"AppSignal extension not loaded. This could mean that your current environment isn't supported, or that another error has occurred."
)
} else {
console.error(
`Failed to load AppSignal extension with error: ${e.message}. Please email us at [email protected] for support.`
)
}
}
extension.start()
}

/**
Expand Down
9 changes: 6 additions & 3 deletions packages/nodejs/src/extension_wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ try {
mod = require("../build/Release/extension.node") as ExtensionWrapper
mod.isLoaded = true
} catch (e) {
console.error(
"AppSignal extension not loaded. This could mean that your current " +
"environment isn't supported, or that another error has occurred."
)

mod = {
isLoaded: false,
extension: {
start() {
throw new Error("Extension module not loaded")
},
start() {},
stop() {},
diagnoseRaw() {},
runningInContainer() {}
Expand Down

0 comments on commit 083a39a

Please sign in to comment.