fix: convert to BigInt before nanosecond multiplication to avoid precision loss#3325
fix: convert to BigInt before nanosecond multiplication to avoid precision loss#3325claygeo wants to merge 1 commit intotriggerdotdev:mainfrom
Conversation
…ision loss Multiplying epoch milliseconds by 1,000,000 as a Number exceeds Number.MAX_SAFE_INTEGER (~9e15), causing IEEE 754 precision loss of ~256ns in ~0.2% of cases. Convert to BigInt first, then multiply. Fixed in 4 locations: - getNowInNanoseconds() in common.server.ts - calculateDurationFromStart() in common.server.ts - recordRunDebugLog() in index.server.ts - retry event recording in runEngineHandlers.server.ts The correct pattern (BigInt(ms) * BigInt(1_000_000)) already existed in convertDateToNanoseconds() in the same file. Closes triggerdotdev#3292
|
|
Hi @claygeo, 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. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThree files in the event repository and run engine handlers were updated to standardize nanoseconds conversion arithmetic. The changes reorder arithmetic operations to convert millisecond timestamps to Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ 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 |
Summary
Closes #3292
Multiplying epoch milliseconds by 1,000,000 as a
NumberexceedsNumber.MAX_SAFE_INTEGER(~9e15), causing IEEE 754 precision loss of ~256ns in ~0.2% of cases. The fix converts toBigIntfirst, then multiplies.Changes
4 locations fixed across 3 files:
common.server.tsgetNowInNanoseconds()common.server.tscalculateDurationFromStart()index.server.tsrecordRunDebugLog()runEngineHandlers.server.tsThe correct pattern already existed in
convertDateToNanoseconds()in the same file — these locations just weren't using it.Test plan