Repository navigation
Conversation
Multiplying Date.getTime() by 1_000_000 as a float before converting to BigInt causes IEEE 754 precision loss once the product exceeds Number.MAX_SAFE_INTEGER (~9007T, which ms * 1e6 hits around 2255 AD). The correct pattern already exists in convertDateToNanoseconds(): BigInt(date.getTime()) * BigInt(1_000_000) Apply it consistently to the three call sites that still use the float-multiply pattern (getNowInNanoseconds, calculateDurationFromStart, and the runEngineHandlers retry-event recordEvent call).
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @okxint, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
| type: fix | ||
| --- | ||
|
|
||
| Fix OTLP trace timestamps losing precision for runs after approximately 2255 AD. Timestamps are now computed with full 64-bit integer arithmetic. |
There was a problem hiding this comment.
What
Three call sites were computing nanosecond timestamps by multiplying
Date.getTime()(a JS float) by1_000_000before converting toBigInt:Once the float product exceeds
Number.MAX_SAFE_INTEGER(~9.007 × 10¹⁵), IEEE 754 precision is lost and theBigIntreceives a silently wrong value.Date.now() * 1_000_000crosses that boundary in roughly the year 2255, but it also means any computed duration past ~9 years of nanoseconds would silently truncate.The correct pattern already exists in the same file (
convertDateToNanoseconds) and avoids the float multiply entirely:Changes
eventRepository/common.server.ts:getNowInNanosecondsandcalculateDurationFromStarteventRepository/index.server.ts: run-eventstartTimeon the replay pathrunEngineHandlers.server.ts: retry-eventstartTimeonrecordEventAll four sites now match the pattern in
convertDateToNanoseconds.Testing
No behavior change for current timestamps — the values are identical for any millisecond value representable today. The fix only affects correctness for values beyond
Number.MAX_SAFE_INTEGER / 1_000_000ms, which isn't reachable in practice, but using integer arithmetic throughout is the right thing to do.