Skip to content

Commit

Permalink
Fix datamap.setFloat implementation
Browse files Browse the repository at this point in the history
The existing implementation for `datamap.setFloat` mistakenly
attempted to use the second argument as both the value to add to
the map and the map to add it to. This resulted in an "Invalid
argument" error thrown when trying to cast the value as a map.

The tests for `Data.generate` tested sending floating point values
to the agent using the value `99.0`. Since JavaScript has a unified
`number` type for integers and floats alike, `99.0` is the same
number as `99`.

The implementation of `Data.generate` decides whether to call
`datamap.setInteger` or `datamap.setFloat` based on whether
`Number.isInteger` returns `true` or `false` -- and it will return
`true` for `99.0`, as it would for `99`. This means that, in
practice, `datamap.setFloat` was not covered by the tests.
  • Loading branch information
unflxw committed Mar 31, 2022
1 parent 3fb6b3a commit 17c0deb
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "fix"
---

Fix an issue where the AppSignal extension would throw an error when an object containing a non-integer number is sent to it. This would be triggered when calling `setSampleData` with an object containing a non-integer number, or when the values for a metric's tags are non-integer numbers.
2 changes: 1 addition & 1 deletion packages/nodejs/ext/appsignal_extension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ Napi::Value SetFloatToDataMap(const Napi::CallbackInfo &info) {
Napi::Number num = info[1].As<Napi::Number>();

Napi::External<appsignal_data_t> map =
info[1].As<Napi::External<appsignal_data_t>>();
info[2].As<Napi::External<appsignal_data_t>>();

appsignal_data_map_set_float(map.Data(), MakeAppsignalString(key_utf8),
num.DoubleValue());
Expand Down
4 changes: 2 additions & 2 deletions packages/nodejs/src/__tests__/data.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe("Data", () => {
const nested = {
string: "payload",
int: 9999,
float: 99.0,
float: 99.9,
1: true,
null: "null_key",
null_value: null,
Expand Down Expand Up @@ -57,7 +57,7 @@ describe("Data", () => {
false,
"string",
9999,
99.0,
99.9,
[1, 2, 3],
{ foo: "bʊr" },
{ arr: [1, 2, "three"], foo: "bʊr" }
Expand Down

0 comments on commit 17c0deb

Please sign in to comment.