Skip to content

STRATCONN-4121 - [Customerio] - Support ISO timestamps with >3 fractional second digits - #3923

Closed
sydneycollins-cio wants to merge 2 commits into
segmentio:mainfrom
customerio:fix/customerio-timestamp-sub-millisecond-precision-v2
Closed

STRATCONN-4121 - [Customerio] - Support ISO timestamps with >3 fractional second digits#3923
sydneycollins-cio wants to merge 2 commits into
segmentio:mainfrom
customerio:fix/customerio-timestamp-sub-millisecond-precision-v2

Conversation

@sydneycollins-cio

Copy link
Copy Markdown
Contributor

Problem

The convertAttributeTimestamps and convertValidTimestamp functions pass ISO timestamp strings directly to dayjs for parsing. dayjs only reliably handles up to 3 fractional second digits (milliseconds). Timestamps with 7+ fractional digits (e.g. 2024-08-14T20:36:48.6527521Z) are silently not converted to Unix format, causing the raw string to be sent downstream instead of the expected integer.

Reported via STRATCONN-4121 by a Twilio partner customer.

Fix

Add normalizeIsoFractionalSeconds() — a one-liner that trims fractional seconds to 3 digits before passing to dayjs. Applied in both conversion functions. No precision is lost since Unix timestamps are second-level.

Tests

Added test cases covering:

  • 7-digit fractional seconds (the reported case)
  • 9-digit fractional seconds (nanoseconds)
  • 3-digit regression (existing behavior preserved)
  • Non-date string passthrough (no regression)

dayjs only reliably parses up to 3 fractional second digits (milliseconds).
Timestamps with 7-9 digits (e.g. 2024-08-14T20:36:48.6527521Z) were silently
not converted to Unix format, causing downstream type errors.

Add normalizeIsoFractionalSeconds() to trim sub-millisecond digits before
dayjs parsing. Applied in both convertAttributeTimestamps and
convertValidTimestamp. No precision is lost since Unix is second-level.

Reported via STRATCONN-4121.
@sydneycollins-cio
sydneycollins-cio requested a review from a team as a code owner August 4, 2026 17:34
@joe-ayoub-segment joe-ayoub-segment self-assigned this Aug 7, 2026
@joe-ayoub-segment joe-ayoub-segment changed the title fix(customerio): support ISO timestamps with >3 fractional second digits STRATCONN-4121 - [Customerio] - Support ISO timestamps with >3 fractional second digits Aug 7, 2026
@joe-ayoub-segment
joe-ayoub-segment requested a lite review from Copilot August 7, 2026 10:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds support for ISO-8601 timestamps containing more than 3 fractional-second digits (sub-millisecond precision) by normalizing them before parsing with dayjs, preventing silent “non-conversion” to Unix timestamps in Customer.io payload handling.

Changes:

  • Introduced normalizeIsoFractionalSeconds() to trim ISO fractional seconds to 3 digits before parsing.
  • Applied normalization in convertValidTimestamp and convertAttributeTimestamps.
  • Added Jest tests for convertAttributeTimestamps covering 7/9-digit fractional seconds and regressions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/destination-actions/src/destinations/customerio/utils.ts Normalizes ISO fractional seconds before dayjs parsing in timestamp conversion helpers.
packages/destination-actions/src/destinations/customerio/tests/utils.test.ts Adds test coverage for convertAttributeTimestamps with higher-precision fractional seconds.

Comment thread packages/destination-actions/src/destinations/customerio/utils.ts
Comment thread packages/destination-actions/src/destinations/customerio/utils.ts
@joe-ayoub-segment

joe-ayoub-segment commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for picking this up! 🙏 I dug into the two screenshots on STRATCONN-4121 and cloned the repo to test — I think this fix lands one step too late to actually resolve the issue.

Repro / root cause

In convertAttributeTimestamps, the conversion only runs inside if (isIsoDate(value)), and isIsoDate ends with !isNaN(Date.parse(value)). In Segment's production runtime, Date.parse returns NaN for fractional seconds longer than ~5 digits, so isIsoDate("2024-08-14T20:36:48.6527521Z") is false — the value skips conversion and passes through as the raw string. That's exactly the 5-vs-7-digit boundary the customer reported.

Because normalizeIsoFractionalSeconds is called inside that block but the gate is checked on the original un-normalized value, the 7-digit case never enters the block and the normalize call is never reached. I confirmed this locally: with this PR applied, the 7- and 9-digit inputs still come out as the raw string. (The current tests pass only because modern Node's Date.parse accepts long fractions, which hides the bug.)

Suggested fix — normalize before the isIsoDate gate

if (typeof value === 'string') {
  const normalized = normalizeIsoFractionalSeconds(value)
  if (isIsoDate(normalized)) {
    ;(clone[key] as unknown) = dayjs(normalized).unix()
    return
  }
}

Could you also add a test that reproduces the failure?

Since local Node parses long fractions fine, the test needs to mock Date.parse to emulate the prod runtime. This one fails against main (and this PR) and passes with the reorder above — I ran all three states locally with the repo's jest to confirm:

describe('convertAttributeTimestamps — sub-millisecond fractional seconds (STRATCONN-4121)', () => {
  const realParse = Date.parse
  afterEach(() => jest.restoreAllMocks())

  it('converts a 7-digit fractional second timestamp when Date.parse rejects long fractions (prod runtime)', () => {
    jest.spyOn(Date, 'parse').mockImplementation((s: string) => {
      const m = /\.(\d+)/.exec(s)
      return m && m[1].length > 5 ? NaN : realParse(s)
    })
    const result = convertAttributeTimestamps({ createdat: '2024-08-14T20:36:48.6527521Z' })
    expect(result.createdat).toBe(1723667808)
  })
})

Oh yes one more think looks like some CI checks are failing.

Thanks for working on this.
Cheers,
Joe

@joe-ayoub-segment

Copy link
Copy Markdown
Contributor

Superseded by #3930

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants