fix(client): decode RESP3 doubles to the value the server sent - #3393
Open
Jaybhade wants to merge 2 commits into
Open
fix(client): decode RESP3 doubles to the value the server sent#3393Jaybhade wants to merge 2 commits into
Jaybhade wants to merge 2 commits into
Conversation
The RESP3 double decoder built its result by adding each fraction digit multiplied by a precalculated power of ten, then applying any exponent as a separate multiplication. Every one of those steps rounds, so the decoded double drifted from the one the server encoded, and large/small magnitudes overflowed or underflowed outright. Digits are now accumulated into an integer significand and scaled by a single division by an exactly representable power of ten, so the result is rounded once — i.e. correctly rounded. Tokens that cannot be reconstructed that way (an `e` exponent, a significand or decimal exponent outside the exact range, `inf`/`nan`) are converted with `Number`, matching what the RESP2 double transformer already did. Scanning stays in place over the chunk, so a double that arrives whole costs no slice and no intermediate string.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nkaradzhov
approved these changes
Aug 7, 2026
nkaradzhov
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @Jaybhade, nice catch, this looks good!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
RESP3 doubles are decoded to a different double than the server sent, for most values. On
master(RESP3 is the default since #3215):Over random doubles round-tripped through the wire format redis actually
writes, 51.5% (3087/5999) came back as a different double than the one
stored. The same
ZSCOREunderRESP: 2is exact, because the RESP2 pathgoes through
transformDoubleReply[2], which just callsNumber().The cause is in
#decodeDouble. Fraction digits were summed asdouble += digit * 1e-kfrom a precalculated multiplier table, and anyexponent was then applied as a separate
double * 10 ** exponent. Each ofthose steps rounds, so the error compounds — and for magnitudes near the
limits of the type the final multiplication overflows to
Infinityorunderflows to
0even though the value itself is perfectly representable.Digits past the 17th were dropped rather than rounded.
This affects everything that returns a RESP3 double:
ZSCORE,ZMSCORE,ZINCRBY,ZADD … INCR,*WITHSCORES,GEODIST,GEOPOS, and themodule commands built on doubles.
What changed
Digits now accumulate into an integer significand and are scaled by a
single division by an exactly representable power of ten. One IEEE
operation on two exact operands rounds once, so the result is the correctly
rounded double — the exact value the server encoded.
Tokens that cannot be rebuilt that way — an
eexponent, a significandabove
MAX_SAFE_INTEGER, more than 22 fraction digits,inf/nan— handoff to
Number(), which is correctly rounded for every input. That fallbackalso fixes the overflow/underflow cases, and its
inf/+inf/-inf/nanhandling is copied from
transformDoubleReply[2]so the two protocolversions can no longer disagree.
Scanning still happens in place over the chunk: a double that arrives whole
costs no slice and no intermediate string. The change is a net −120 lines,
since the exponent and decimal continuation state machines are gone; the
split-across-chunks case buffers the token and converts it once whole.
Verification
Wire bytes for the differential tests were produced by compiling redis's own
deps/fpconv/fpconv_dtoa.cand driving it exactly asd2string()does, sothe inputs are the literal bytes the server writes, not an approximation.
uniform, log-uniform, and 2-decimal): before 3098 wrong, after 0.
1.2.3,0x10,' 1', bare+,1e, 30-digit significands): the fast path agrees withNumber()on all of them, 0 divergences.byte-by-byte, and random multi-cut) over 21 doubles — 0 failures.
decoder.spec.ts; each runs single-chunk and byte-by-bytevia the existing
test()helper. 18 of the 22 assertions fail onmaster.npm run buildandnpm run test:types -w @redis/clientclean;eslint --max-warnings=0clean on both changed files.packages/client: 344 passing. I could not run thedockerized suite (no host-networking Docker on this machine), so the
integration tests are unverified locally — CI covers them.
Performance
This file is written for speed, so I measured old vs new in the same process
with rounds interleaved, reporting medians and spreads rather than a single
delta:
ZRANGE … WITHSCORES, 500 members, RESP3eexponents and extremes768k doubles per run. Realistic replies are unchanged; the 1.16x is a stream
made entirely of shapes that now take the
Number()fallback, which nolonger decodes them wrongly.
Checklist
npm testpass with this change (including linting)? Build, typetests, lint and all Docker-free specs pass; the dockerized suite needs
host networking I don't have locally.
Note
Medium Risk
Touches hot-path protocol parsing used by all RESP3 double replies (scores, geo, modules); behavior change is correctness-focused but affects every consumer of those values.
Overview
Fixes incorrect RESP3 double decoding so values like sorted-set scores and geo coordinates match what Redis encoded on the wire (e.g.
0.3,99.99,Number.MAX_VALUEno longer drift, overflow toInfinity, or underflow to0).#decodeDoublein the RESP decoder replaces the old digit-by-digit decimal multipliers and separate exponent scaling with an integer significand scaled by one division using exactly representable powers of ten. Tokens that cannot use that path (eexponents, huge significands,inf/nan, etc.) go through a newslowParseDoublehelper aligned with the RESP2Number()transformer.Tests: adds a
round-trips the double the server encodedblock indecoder.spec.ts(including byte-by-byte chunk splits via the existing harness).Reviewed by Cursor Bugbot for commit cf3857e. Bugbot is set up for automated code reviews on this repo. Configure here.