bug(util-dynamodb): BigInt value increases by 1n after marshall+unmarshall

[trivikr]

Describe the bug
BigInt value increases by 1n after marshall+unmarshall

SDK version number
@aws-sdk/util-dynamodb@1.0.0-gamma.1

Is the issue in the browser/Node.js/ReactNative?
Node.js

Details of the browser/Node.js/ReactNative version
v12.18.3

To Reproduce (observed behavior)

Code

const { deepStrictEqual } = require("assert");
const { marshall, unmarshall } = require("@aws-sdk/util-dynamodb");

const bigInt = 9007199254740995n;
deepStrictEqual(bigInt, unmarshall(marshall({ bigInt })));

Output

assert.js:100
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

+ 9007199254740995n
- {
-   bigInt: 9007199254740996n
- }
    at Object.<anonymous> (/local/home/trivikr/workspace/ddb-test/bigint.js:5:1)
    at Module._compile (internal/modules/cjs/loader.js:1137:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1157:10)
    at Module.load (internal/modules/cjs/loader.js:985:32)
    at Function.Module._load (internal/modules/cjs/loader.js:878:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
    at internal/main/run_main_module.js:17:47 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 9007199254740995n,
  expected: { bigInt: 9007199254740996n },
  operator: 'deepStrictEqual'
}

Expected behavior
Value of BigInt doesn't change while marshalling/unmarshalling. i.e. No AssertionError

[trivikr]

This happens because Number value for a BigInt string if one more than what's passed to it.

> Number("9007199254740995")
9007199254740996
> BigInt(Number("9007199254740995"))
9007199254740996n

This conversion is used in code at

aws-sdk-js-v3/packages/util-dynamodb/src/convertToNative.ts

Lines 49 to 53 in 1b1e40d

	const num = Number(numString);
	const infinityValues = [Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY];
	if ((num > Number.MAX_SAFE_INTEGER || num < Number.MIN_SAFE_INTEGER) && !infinityValues.includes(num)) {
	if (typeof BigInt === "function") {
	return BigInt(num);

Fix: use BigInt on number string instead:

> BigInt("9007199254740995")
9007199254740995n

[TechTeam12]

So, the effective code change would be to change line 53 to BigInt(numString) from BigInt(num)?

Also, to note; Number conversions of BigInt strings doesn't always produce a +1 result. For example
Number("9007199254740996") will result in 9007199254740996
In addition Number("9007199254740997") will also result in 9007199254740996. This is because big ints are not safe values in integer space.

[trivikr]

So, the effective code change would be to change line 53 to BigInt(numString) from BigInt(num)?

Yup, there's an open PR at #1544

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.