Skip to content

Commit

Permalink
Fix boolean config options set to false
Browse files Browse the repository at this point in the history
When a boolean config option was set to false when initializing the
client and it wasn't its default value, it wouldn't work. This was due
to they way we checked the existence of values when setting the private
env vars. As it was a regular `if`, a `false` value would always be
`false` and the transformation skipped.

This commit checks if the present config value is of boolean type and
transforms it into a string nevermind its value.
  • Loading branch information
luismiramirez committed Feb 1, 2024
1 parent 7005265 commit c41d2f1
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 6 deletions.
6 changes: 6 additions & 0 deletions .changesets/fix-boolean-config-options.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "fix"
---

Fix issue with boolean config options not being set to `false` properly for the agent configuration.
8 changes: 4 additions & 4 deletions src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,16 +256,16 @@ describe("Configuration", () => {
})

it("writes default configuration values to the environment", () => {
expect(env("_APPSIGNAL_ACTIVE")).toBeUndefined()
expect(env("_APPSIGNAL_ACTIVE")).toEqual("false")
expect(env("_APPSIGNAL_APP_ENV")).toBeDefined()
expect(env("_APPSIGNAL_APP_NAME")).toBeUndefined()
expect(env("_APPSIGNAL_BIND_ADDRESS")).toBeUndefined()
expect(env("_APPSIGNAL_CA_FILE_PATH")).toMatch(/cert\/cacert\.pem$/)
expect(env("_APPSIGNAL_DNS_SERVERS")).toBeUndefined()
expect(env("_APPSIGNAL_ENABLE_HOST_METRICS")).toEqual("true")
expect(env("_APPSIGNAL_ENABLE_OPENTELEMETRY_HTTP")).toEqual("true")
expect(env("_APPSIGNAL_ENABLE_STATSD")).toBeUndefined()
expect(env("_APPSIGNAL_ENABLE_NGINX_METRICS")).toBeUndefined()
expect(env("_APPSIGNAL_ENABLE_STATSD")).toEqual("false")
expect(env("_APPSIGNAL_ENABLE_NGINX_METRICS")).toEqual("false")
expect(env("_APPSIGNAL_FILES_WORLD_ACCESSIBLE")).toEqual("true")
expect(env("_APPSIGNAL_FILTER_DATA_KEYS")).toBeUndefined()
expect(env("_APPSIGNAL_HOSTNAME")).toBeUndefined()
Expand Down Expand Up @@ -338,7 +338,7 @@ describe("Configuration", () => {
expect(env("_APPSIGNAL_APP_NAME")).toEqual(name)
expect(env("_APPSIGNAL_BIND_ADDRESS")).toEqual("0.0.0.0")
expect(env("_APPSIGNAL_DNS_SERVERS")).toEqual("8.8.8.8,8.8.4.4")
expect(env("_APPSIGNAL_ENABLE_HOST_METRICS")).toEqual("true")
expect(env("_APPSIGNAL_ENABLE_HOST_METRICS")).toEqual("false")
expect(env("_APPSIGNAL_ENABLE_OPENTELEMETRY_HTTP")).toEqual("false")
expect(env("_APPSIGNAL_ENABLE_STATSD")).toEqual("true")
expect(env("_APPSIGNAL_ENABLE_NGINX_METRICS")).toEqual("true")
Expand Down
2 changes: 1 addition & 1 deletion src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,6 @@ export class Client {
console.error(
"The `appsignal.tracer()` function was called, but it has been removed in AppSignal for Node.js package version 3.x. Please read our migration guide to upgrade to this new version of our package: https://docs.appsignal.com/nodejs/3.x/migration-guide.html. It is also possible to downgrade to version 2.x, after which this code will work again."
)
return () => { }
return () => {}
}
}
4 changes: 3 additions & 1 deletion src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,9 @@ export class Configuration {
process.env[k] = current.join(",")
}

if (current) process.env[k] = String(current)
if (current || typeof current === "boolean") {
process.env[k] = String(current)
}
})
}

Expand Down

0 comments on commit c41d2f1

Please sign in to comment.