Skip to content

Commit ee6f6ac

Browse files
committed
protect against NaN values for sample rate
1 parent c7a44b9 commit ee6f6ac

File tree

2 files changed

+20
-3
lines changed

2 files changed

+20
-3
lines changed

packages/tracing/src/hubextensions.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ function getDefaultSampleContext<T extends Transaction>(transaction: T): SampleC
167167
* Checks the given sample rate to make sure it is valid type and value (a boolean, or a number between 0 and 1).
168168
*/
169169
function isValidSampleRate(rate: unknown): boolean {
170-
if (!(typeof rate === 'number' || typeof rate === 'boolean')) {
170+
// we need to check NaN explicitly because it's of type 'number' and therefore wouldn't get caught by this typecheck
171+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
172+
if (isNaN(rate as any) || !(typeof rate === 'number' || typeof rate === 'boolean')) {
171173
logger.warn(
172174
`[Tracing] Given sample rate is invalid. Sample rate must be a boolean or a number between 0 and 1. Got ${JSON.stringify(
173175
rate,

packages/tracing/test/hub.test.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,13 @@ describe('Hub', () => {
192192
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be a boolean or a number'));
193193
});
194194

195+
it('should reject tracesSampleRates which are NaN', () => {
196+
const hub = new Hub(new BrowserClient({ tracesSampleRate: 'dogs!' as any }));
197+
hub.startTransaction({ name: 'dogpark' });
198+
199+
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be a boolean or a number'));
200+
});
201+
195202
// the rate might be a boolean, but for our purposes, false is equivalent to 0 and true is equivalent to 1
196203
it('should reject tracesSampleRates less than 0', () => {
197204
const hub = new Hub(new BrowserClient({ tracesSampleRate: -26 }));
@@ -216,6 +223,14 @@ describe('Hub', () => {
216223
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be a boolean or a number'));
217224
});
218225

226+
it('should reject tracesSampler return values which are NaN', () => {
227+
const tracesSampler = jest.fn().mockReturnValue(NaN);
228+
const hub = new Hub(new BrowserClient({ tracesSampler }));
229+
hub.startTransaction({ name: 'dogpark' });
230+
231+
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be a boolean or a number'));
232+
});
233+
219234
// the rate might be a boolean, but for our purposes, false is equivalent to 0 and true is equivalent to 1
220235
it('should reject tracesSampler return values less than 0', () => {
221236
const tracesSampler = jest.fn().mockReturnValue(-12);
@@ -260,11 +275,11 @@ describe('Hub', () => {
260275
});
261276

262277
it('should propagate sampling decision to child transactions in XHR header', () => {
263-
// TODO this doesn't currently happen, but it should
278+
// TODO fix this and write the test
264279
});
265280

266281
it('should propagate sampling decision to child transactions in fetch header', () => {
267-
// TODO this doesn't currently happen, but it should
282+
// TODO fix this and write the test
268283
});
269284

270285
it("should inherit parent's sampling decision when creating a new transaction if tracesSampler is undefined", () => {

0 commit comments

Comments
 (0)