Skip to content

fix(core): deserialize numeric members to number when the reviver is active - #8247

Open
RasengunShotaro wants to merge 1 commit into
aws:mainfrom
RasengunShotaro:fix/numeric-members-revived-value
Open

fix(core): deserialize numeric members to number when the reviver is active#8247
RasengunShotaro wants to merge 1 commit into
aws:mainfrom
RasengunShotaro:fix/numeric-members-revived-value

Conversation

@RasengunShotaro

Copy link
Copy Markdown

Issue

#8246

Description

needsReviver returns true when a response schema contains a bigInteger, bigDecimal or document member, which enables the JSON.parse reviver for the whole body. jsonReviver then rewrites every numeric literal that does not round-trip (1.0, 0.0, 1e3, anything past Number.MAX_SAFE_INTEGER), including literals that belong to plain numeric members, and the shape deserializer passed those values through unchanged.

The result is that a member whose generated type is number can deserialize to a NumericValue. It fails silently: typeof result.score becomes "object" and result.score + 1 evaluates to the string "1.01" instead of 2. TypeScript cannot catch it, because the generated type says score?: number, so a typeof x === "number" guard looks redundant and is a candidate for removal.

This affects any client whose response shape contains a document member. It was found on bedrock-agent-runtime Retrieve, where RetrievalResultMetadata is Map<String, Document>: the knowledge base returns page numbers as 22.0 inside metadata, and the relevance score in the same response was corrupted as collateral for exact 1.0 / 0.0 values.

Numeric members now coerce a revived NumericValue or BigInt back to number, which matches the generated types and the behaviour before @aws-sdk/core@3.977.0. Documents keep the revived representation, which is unchanged and appears intended.

Applied to both codecs (JsonShapeDeserializer and JsonShapeDeserializer2) so v1/v2 equivalence is preserved.

Testing

Added a test to each codec spec asserting that a numeric member deserializes to number while the reviver is active (1.01, 0.00, 1e31000). Both fail without the change:

× JsonShapeDeserializer > deserializes numeric members to number when the reviver is active
  → expected { scalar: NumericValue{ …(2) } } to deeply equal { scalar: 1 }

Full protocols suite after the change: 32 files, 516 passed, 2 skipped.

One existing expectation is updated. In JsonShapeDeserializer2.spec.ts, fractional exponent with many digits does not throw in reviver asserted that {"scalar": 1.123456789012345678E16} stays a NumericValue. scalar is a numeric member, and JSON.parse produced 11234567890123456 for that input before the reviver existed, so the expectation is changed to that value. The purpose of the test — that the reviver does not throw — is unaffected, and the neighbouring v1/v2 equivalence cases still pass.

I could not run the repo's full yarn install locally, so verification was scoped to packages-internal/core/src/submodules/protocols with dependencies installed from npm.

Checklist

  • If the PR is a feature, add integration tests (*.integ.spec.ts) or E2E tests.
    • It's not a feature.
  • My E2E tests are resilient to concurrent i/o.
    • I didn't write any E2E tests.
  • I added access level annotations e.g. @public, @internal tags and enabled doc generation on the package. Remember that access level annotations go below the description, not above.
    • I didn't add any public functions.
  • Streams - how do they work?? My WebStream readers/locks are properly lifecycled. Node.js stream backpressure is handled. Error handling.
    • No streams here.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…active

`needsReviver` returns true when a response schema contains a bigInteger,
bigDecimal or document member, which enables the JSON.parse reviver for the
whole body. `jsonReviver` then rewrites every numeric literal that does not
round-trip, including literals belonging to plain numeric members, and the
shape deserializer passed those through unchanged.

As a result a member whose generated type is `number` could deserialize to a
`NumericValue`. This is silent: `typeof score` becomes `"object"` and
`score + 1` evaluates to the string `"1.01"` instead of `2`.

Numeric members now coerce a revived `NumericValue` or `BigInt` back to
`number`, matching the generated types and the behaviour prior to 3.977.0.
Documents keep the revived representation, which is unchanged.

One existing expectation is updated: `scalar` is a numeric member, so
`1.123456789012345678E16` now deserializes to `11234567890123456`, which is
what `JSON.parse` produced before the reviver was introduced. That test guards
against throwing in the reviver, and it still does.
@RasengunShotaro
RasengunShotaro force-pushed the fix/numeric-members-revived-value branch from 276e7e7 to a08ef68 Compare August 4, 2026 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant