STRATCONN-4121 - [Customerio] - fix timestamp normalization occurring after isIsoDate gate - #3930
Open
sydneycollins-cio wants to merge 2 commits into
Conversation
…te (STRATCONN-4121) Joe Ayoub's review of PR segmentio#3923 identified that the fix landed one step too late: convertAttributeTimestamps called isIsoDate(value) on the original string, but in Segment's prod runtime Date.parse returns NaN for >5 fractional digits — so 7-digit timestamps like 2024-08-14T20:36:48.6527521Z never entered the normalization block. Fix: normalize before the gate (normalize -> isIsoDate -> dayjs) so the isIsoDate check sees a valid 3-digit millisecond string regardless of the original precision. Also applies normalizeIsoFractionalSeconds to convertValidTimestamp (same root cause). Tests: adds mocked Date.parse cases that emulate prod runtime behavior, confirming the fix works even when Date.parse rejects long fractions.
joe-ayoub-segment
approved these changes
Aug 7, 2026
Contributor
|
Thanks @sydneycollins-cio approved to be deployed. Thank you! |
sydneycollins-cio
force-pushed
the
fix/customerio-timestamp-normalize-before-gate
branch
from
August 7, 2026 15:14
f0e14d2 to
b3cce8f
Compare
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.
Problem
Supersedes PR #3923. Joe Ayoub identified that the fix in #3923 lands one step too late.
In
convertAttributeTimestamps, the code callsisIsoDate(value)on the original string before normalizing.isIsoDateends with!isNaN(Date.parse(value)). In Segment's prod runtime,Date.parsereturnsNaNfor fractional seconds longer than ~5 digits — so a 7-digit timestamp like2024-08-14T20:36:48.6527521ZfailsisIsoDate, never enters the block, and normalization is never reached. Tests in #3923 passed only because local Node accepts long fractions.Fix
Move
normalizeIsoFractionalSecondsto before theisIsoDategate inconvertAttributeTimestamps:Same fix applied to
convertValidTimestamp.normalizeIsoFractionalSecondstrims fractional seconds to 3 digits — no precision is lost since Unix timestamps are second-level.Tests
Added mocked
Date.parsecases that emulate Segment's prod runtime behavior (reject fractions longer than 5 digits), confirming the fix works in that environment. All 20 tests pass.