Skip to content

Commit

Permalink
Fix extension function fallbacks (#557)
Browse files Browse the repository at this point in the history
As reported in #556, when the extension is not installed or fails to
load, calls to certain extension functions will fail. The issue mentions
the `runningInContainer` broken being broken on the main branch.

PR #554 also removes some safety checks that cause the same issue for
the `diagnoseRaw` function. Those safety checks were removed because
they did not what expected, tracking if the extension was running or
not.

For both functions I've implemented a no-operations function. This will
fix any errors on calls to these functions. Other functions that call
these functions need to handle receiving no return value.

Part of #556, but doesn't fix it entirely. We should call more function
calls. These ones were the most obvious ones.

Based on PR #554.
  • Loading branch information
tombruijn authored Jan 19, 2022
1 parent 56fb794 commit f52a824
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "fix"
---

Fix the extension function fallbacks on installation failure. When the extension fails to install and calls are made to the not loaded functions, it will no longer throw an error.
11 changes: 11 additions & 0 deletions packages/nodejs/src/__tests__/extension.failure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,15 @@ describe("Extension", () => {
ext.stop()
}).not.toThrow()
})

it("does not error on diagnoseRaw", () => {
expect(ext.diagnose()).toMatchObject({
error: expect.any(Error),
output: [""]
})
})

it("does not error on runningInContainer", () => {
expect(ext.runningInContainer()).toBeUndefined()
})
})
2 changes: 1 addition & 1 deletion packages/nodejs/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class Extension {
} catch (error) {
return {
error: error,
output: diagnostics_report_string.split("\n")
output: (diagnostics_report_string || "").split("\n")
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/nodejs/src/extension_wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ try {
start() {
throw new Error("Extension module not loaded")
},
stop() {
return
}
stop() {},
diagnoseRaw() {},
runningInContainer() {}
}
} as ExtensionWrapper
}
Expand Down

0 comments on commit f52a824

Please sign in to comment.