Skip to content

Commit 96645e1

Browse files
Return 400 when a timeseries range is too long for the forced interval (#2)
Fixes #1
1 parent 84b684b commit 96645e1

4 files changed

Lines changed: 57 additions & 7 deletions

File tree

‎packages/core/src/time.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ export function nextBucket(bucket: number, granularity: Granularity, timeZone =
8686
}
8787
}
8888

89+
/** Most buckets {@link bucketsBetween} returns before it throws. */
90+
export const MAX_BUCKETS = 10_000;
91+
8992
/** Bucket starts covering the half-open interval [from, to). */
9093
export function bucketsBetween(
9194
from: number,
@@ -100,7 +103,8 @@ export function bucketsBetween(
100103
b = nextBucket(b, granularity, timeZone)
101104
) {
102105
out.push(b);
103-
if (out.length > 10_000) throw new RangeError('Range too large for the chosen granularity');
106+
if (out.length > MAX_BUCKETS)
107+
throw new RangeError('Range too large for the chosen granularity');
104108
}
105109
return out;
106110
}

‎server/src/routes/schemas.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import { DAY, DIMENSIONS, type Dimension, type Granularity } from '@pulse/core';
1+
import {
2+
DAY,
3+
DIMENSIONS,
4+
HOUR,
5+
MAX_BUCKETS,
6+
MINUTE,
7+
type Dimension,
8+
type Granularity,
9+
} from '@pulse/core';
210

311
export const rangeQuerySchema = {
412
type: 'object',
@@ -61,3 +69,17 @@ export function resolveRange(q: RangeQuerystring, now: number): { from: number;
6169
if (from >= to) return { from: to - DAY, to };
6270
return { from: Math.max(from, to - 400 * DAY), to };
6371
}
72+
73+
const BUCKET_MS: Record<Granularity, number> = { minute: MINUTE, hour: HOUR, day: DAY };
74+
75+
/**
76+
* Longest range, in buckets, that a forced `interval` may cover. One less than
77+
* MAX_BUCKETS because `from` snaps down to a bucket start, so a range n buckets
78+
* long can touch n + 1 of them. With the 400-day clamp above, only `minute` can
79+
* hit it.
80+
*/
81+
export const MAX_INTERVAL_SPAN = MAX_BUCKETS - 1;
82+
83+
export function fitsInterval(range: { from: number; to: number }, interval: Granularity): boolean {
84+
return range.to - range.from <= MAX_INTERVAL_SPAN * BUCKET_MS[interval];
85+
}

‎server/src/routes/stats.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ import type { FastifyInstance, FastifyReply } from 'fastify';
22
import type { AppContext } from '../context.js';
33
import { toCsv } from '../stats/csv.js';
44
import {
5+
MAX_INTERVAL_SPAN,
56
breakdownQuerySchema,
7+
fitsInterval,
68
rangeQuerySchema,
79
resolveRange,
810
siteParamsSchema,
@@ -38,11 +40,14 @@ export function statsRoutes(app: FastifyInstance, ctx: AppContext): void {
3840
{ schema: { params: siteParamsSchema, querystring: timeseriesQuerySchema } },
3941
async (req, reply) => {
4042
if (!ensureSite(req.params.siteId, reply)) return reply;
41-
return stats.timeseries(
42-
req.params.siteId,
43-
resolveRange(req.query, ctx.now()),
44-
req.query.interval,
45-
);
43+
const range = resolveRange(req.query, ctx.now());
44+
const { interval } = req.query;
45+
if (interval && !fitsInterval(range, interval)) {
46+
return reply.code(400).send({
47+
error: `interval=${interval} covers at most ${MAX_INTERVAL_SPAN} ${interval}s; use a shorter range or a coarser interval`,
48+
});
49+
}
50+
return stats.timeseries(req.params.siteId, range, interval);
4651
},
4752
);
4853

‎server/test/api.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {
2+
DAY,
23
HOUR,
34
MINUTE,
5+
SECOND,
46
type BreakdownResponse,
57
type RealtimeResponse,
68
type SummaryResponse,
@@ -113,6 +115,23 @@ describe('stats API', () => {
113115
);
114116
});
115117

118+
it('answers 400 when a forced interval needs too many buckets', async () => {
119+
const minute = (from: number, to: number) =>
120+
t.app.inject(`/api/sites/demo/timeseries?from=${from}&to=${to}&interval=minute`);
121+
// The ranges from issue #1: 6 days worked, 8 days answered 500.
122+
expect((await minute(NOW - 6 * DAY, NOW)).statusCode).toBe(200);
123+
const tooLong = await minute(NOW - 8 * DAY, NOW);
124+
expect(tooLong.statusCode).toBe(400);
125+
expect(tooLong.json()).toEqual({ error: expect.stringContaining('interval=minute') });
126+
// Exact boundary. `from` is off the minute grid and `to` is past now, so the
127+
// series gets an extra partial bucket at the front and nothing is trimmed at the end.
128+
const to = NOW + 30 * SECOND;
129+
const longest = await minute(to - 9_999 * MINUTE, to);
130+
expect(longest.statusCode).toBe(200);
131+
expect(longest.json<TimeseriesResponse>().points).toHaveLength(10_000);
132+
expect((await minute(to - 10_000 * MINUTE, to)).statusCode).toBe(400);
133+
});
134+
116135
it('breaks down by page, referrer, country and device', async () => {
117136
const range = `from=${NOW - 24 * HOUR}&to=${NOW + MINUTE}`;
118137
const pages = (

0 commit comments

Comments
 (0)