Repository navigation
(fix) tracker-js: apply error sampleRate the right way round - #615
sarmah-rup wants to merge 1 commit into
Conversation
captureError skipped an error when sampleRate >= Math.random(), so sampleRate: 1 dropped every error and sampleRate: 0 sent all of them. Skip when Math.random() >= sampleRate instead, which matches the documented meaning (share of errors to send).
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. π βΉοΈ Recent review infoβοΈ Run configuration
π Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. π WalkthroughWalkthrough
ChangesError sampling
Priority: β Normal Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Β· Severity of issue fixed: Medium Suggested reviewers: Merge Risk: βͺ Minimal Β· up to Automatic error sampling now follows the configured rate. No merge-blocking issue is identified. Security Architecture ReviewSecurity architecture risk: π΅ Low Β· up to The fix restores the requested error sampling behavior without changing payload fields, destinations, or callback vetoes. Explicit high sampling rates can now transmit substantially more error data. No introduced vulnerability was established, but server-side handling was not verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
π₯ Pre-merge checks | β 5β Passed checks (5 passed)
β¨ 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 |
Changes
Fixes #614.
captureErrorskipped an error whensampleRate >= Math.random(), sosampleRate: 1(the default) dropped every automatically captured error andsampleRate: 0sent all of them. I flipped the comparison sosampleRateis the share of errors that get sent, as the option's doc comment describes. No breaking changes, except that setups which relied on the inverted behaviour will now send the share they asked for.Testing
Added
packages/tracker-js/tests/errorSampling.test.ts. It uses the realLibwithMath.randomstubbed and covers rates 1, 0 and 0.2. All 3 fail on main and pass with the fix. The full tracker-js suite passes (11 suites, 89 tests), andtsc --noEmitand prettier are clean.AI assistance
Claude (Anthropic) helped me trace the bug and write the test. I reviewed the change and ran the tests myself.
Checklist
Summary by CodeRabbit