Skip to content

Commit

Permalink
Implement config validation (#482)
Browse files Browse the repository at this point in the history
Config was always considered to be valid as `Config.isValid()` just had
a `return true` inside. Now we check the presence of an apiKey in the
configuration data to validate the config.
  • Loading branch information
luismiramirez authored Nov 4, 2021
1 parent c750216 commit 2ceda25
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 20 deletions.
32 changes: 16 additions & 16 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
bump: "patch"
---

The AppSignal config is not considered valid anymore when the apiKey config option is not set.
6 changes: 6 additions & 0 deletions packages/nodejs/src/__tests__/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ describe("BaseClient", () => {
expect(client.isActive).toBeFalsy()
})

it("does not start the client if config is not valid", () => {
process.env["APPSIGNAL_PUSH_API_KEY"] = undefined
client = new BaseClient({ name, enableMinutelyProbes: false })
expect(client.isActive).toBeFalsy()
})

it("starts the client when the active option is true", () => {
client = new BaseClient({ ...DEFAULT_OPTS, active: true })
expect(client.isActive).toBeTruthy()
Expand Down
22 changes: 22 additions & 0 deletions packages/nodejs/src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,28 @@ describe("Configuration", () => {
})
})

describe(".isValid", () => {
it("is valid if apiKey is present", () => {
expect(config.isValid).toBeTruthy()
})

it("is invalid if apiKey is not present", () => {
process.env["APPSIGNAL_PUSH_API_KEY"] = undefined
config = new Configuration({ name })
expect(config.isValid).toBeFalsy()
})

it("is invalid if apiKey is an empty string", () => {
config = new Configuration({ name, apiKey: "" })
expect(config.isValid).toBeFalsy()
})

it("is invalid if apiKey is a string with only whitespaces", () => {
config = new Configuration({ name, apiKey: " " })
expect(config.isValid).toBeFalsy()
})
})

describe("Overriden log path with file specified", () => {
beforeEach(() => {
process.env["APPSIGNAL_LOG_PATH"] = "/other_path/appsignal.log"
Expand Down
2 changes: 1 addition & 1 deletion packages/nodejs/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class BaseClient implements Client {
if (this.config.isValid) {
this.extension.start()
} else {
console.error("Not starting, no valid config for this environment")
console.error("Not starting, no valid AppSignal configuration found")
}
}

Expand Down
4 changes: 1 addition & 3 deletions packages/nodejs/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,9 @@ export class Configuration {

/**
* Returns `true` if the current configuration is valid.
*
* @todo
*/
public get isValid(): boolean {
return true
return (this.data.apiKey || "").trim() !== ""
}

/**
Expand Down

0 comments on commit 2ceda25

Please sign in to comment.