Ryan/documenso
1
0
Fork 1
mirror of https://github.com/documenso/documenso.git synced 2026-06-22 04:12:06 +10:00
Code Issues Packages Projects Releases Wiki Activity

fix: track monthly usage for unlimited quotas (#2894)

Browse Source
This commit is contained in:
David Nguyen
2026-05-31 13:34:10 +10:00
committed by GitHub
parent 61138cdd81
commit 44c4826e92
3 changed files with 41 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/app-tests/e2e/api/v1/organisation-rate-limits.spec.ts
+15 -9
View File
@@ -168,7 +168,8 @@ const getMonthlyStat = async (organisation: Organisation) =>
* The DB counter is the source of truth for quota enforcement, so checking its
* exact value (not just the HTTP response) proves the documented increment
* semantics in check-monthly-quota.ts:
* - quota === null -> never incremented (stays 0)
* - quota === null -> unlimited: never blocks, but the request is STILL
* counted (the upsert now runs before the null return)
* - quota === 0 -> throws BEFORE increment (stays 0)
* - quota > 0 -> incremented by `count` BEFORE the over-quota check, so
* even the request that gets rejected still advances it
@@ -456,9 +457,10 @@ test.describe('Organisation dynamic rate limits & quotas (API v1)', () => {
// client. We therefore assert no org-specific header is surfaced.
expectNoOrgRateLimitHeader(limitedRes);
// Windowed limiting uses the RateLimit bucket table, NOT the monthly quota
// counter (quota is null here), so apiCount must remain untouched.
await expectMonthlyCounter(organisation, 'apiCount', 0);
// The windowed stage blocks the (MAX+1)th request before the quota upsert,
// but each of the MAX allowed requests still records usage (null quota now
// tracks instead of skipping), so the counter equals MAX.
await expectMonthlyCounter(organisation, 'apiCount', MAX);
});
test('a single allowed request succeeds when the limit is 1', async ({ request }) => {
@@ -476,7 +478,9 @@ test.describe('Organisation dynamic rate limits & quotas (API v1)', () => {
const body = await expectOrgLimited(limitedRes);
expect(String(body.message)).toMatch(WINDOWED_LIMIT_MESSAGE);
await expectMonthlyCounter(organisation, 'apiCount', 0);
// The one allowed request is counted (null quota still tracks); the blocked
// request trips the window before the quota upsert, so the counter is 1.
await expectMonthlyCounter(organisation, 'apiCount', 1);
});
test('the windowed limit RESETS once the window elapses (429 -> wait -> 200)', async ({ request }) => {
@@ -531,8 +535,9 @@ test.describe('Organisation dynamic rate limits & quotas (API v1)', () => {
expect(res.status()).toBe(200);
}
// A null quota means "unlimited" and must never increment the counter.
await expectMonthlyCounter(organisation, 'apiCount', 0);
// A null quota means "unlimited" (never blocks), but every request is now
// recorded so usage is visible on unlimited plans — so the counter is 6.
await expectMonthlyCounter(organisation, 'apiCount', 6);
});
test('exhausting the quota 429s without rate-limit headers and keeps counting', async ({ request }) => {
@@ -603,8 +608,9 @@ test.describe('Organisation dynamic rate limits & quotas (API v1)', () => {
await expectNotGlobalLimited(res);
expect(res.ok(), `resend should succeed: ${await res.text()}`).toBeTruthy();
// Windowed pass with a null quota must NOT touch the monthly counter.
await expectMonthlyCounter(organisation, 'emailCount', 0);
// The windowed pass is now recorded even though the quota is null, so the
// counter advances by the batch size (recipientIds.length).
await expectMonthlyCounter(organisation, 'emailCount', recipientIds.length);
});
test('resend is blocked when recipient count exceeds the email window', async ({ request }) => {
packages/app-tests/e2e/api/v2/organisation-rate-limits.spec.ts
+21 -13
View File
@@ -157,7 +157,8 @@ const getMonthlyStat = async (organisation: Organisation) =>
* The DB counter is the source of truth for quota enforcement, so checking its
* exact value (not just the HTTP response) proves the documented increment
* semantics in check-monthly-quota.ts:
* - quota === null -> never incremented (stays 0)
* - quota === null -> unlimited: never blocks, but the request is STILL
* counted (the upsert now runs before the null return)
* - quota === 0 -> throws BEFORE increment (stays 0)
* - quota > 0 -> incremented by `count` BEFORE the over-quota check, so
* even the request that gets rejected still advances it
@@ -543,9 +544,10 @@ test.describe('Organisation dynamic rate limits & quotas', () => {
expect(limitedRes.headers()['x-ratelimit-remaining']).toContain('0');
expect(limitedRes.headers()['retry-after']).toBeTruthy();
// Windowed limiting uses the RateLimit bucket table, NOT the monthly quota
// counter (quota is null here), so apiCount must remain untouched.
await expectMonthlyCounter(organisation, 'apiCount', 0);
// The windowed stage blocks the (MAX+1)th request before the quota upsert,
// but each of the MAX allowed requests still records usage (null quota now
// tracks instead of skipping), so the counter equals MAX.
await expectMonthlyCounter(organisation, 'apiCount', MAX);
});
test('a single allowed request succeeds when the limit is 1', async ({ request }) => {
@@ -563,7 +565,9 @@ test.describe('Organisation dynamic rate limits & quotas', () => {
const body = await expectOrgLimited(limitedRes);
expect(String(body.message)).toMatch(WINDOWED_LIMIT_MESSAGE);
await expectMonthlyCounter(organisation, 'apiCount', 0);
// The one allowed request is counted (null quota still tracks); the blocked
// request trips the window before the quota upsert, so the counter is 1.
await expectMonthlyCounter(organisation, 'apiCount', 1);
});
test('the windowed limit RESETS once the window elapses (429 -> wait -> 200)', async ({ request }) => {
@@ -618,8 +622,9 @@ test.describe('Organisation dynamic rate limits & quotas', () => {
expect(res.status()).toBe(200);
}
// A null quota means "unlimited" and must never increment the counter.
await expectMonthlyCounter(organisation, 'apiCount', 0);
// A null quota means "unlimited" (never blocks), but every request is now
// recorded so usage is visible on unlimited plans — so the counter is 6.
await expectMonthlyCounter(organisation, 'apiCount', 6);
});
test('exhausting the quota 429s without rate-limit headers and keeps counting', async ({ request }) => {
@@ -699,8 +704,9 @@ test.describe('Organisation dynamic rate limits & quotas', () => {
expectRateLimitHeaderToInclude(limitedRes, MAX);
expect(limitedRes.headers()['retry-after']).toBeTruthy();
// Windowed limiting doesn't touch the quota counter (documentQuota is null).
await expectMonthlyCounter(organisation, 'documentCount', 0);
// The (MAX+1)th create trips the window before the quota upsert, but each of
// the MAX allowed creates still records usage (null quota now tracks).
await expectMonthlyCounter(organisation, 'documentCount', MAX);
});
});
@@ -747,8 +753,9 @@ test.describe('Organisation dynamic rate limits & quotas', () => {
await expectNotGlobalLimited(res);
expect(res.ok()).toBeTruthy();
// A null quota means "unlimited" and must not increment the counter.
await expectMonthlyCounter(organisation, 'documentCount', 0);
// A null quota is unlimited (never blocks) but is now still recorded, so the
// single create advances the counter to 1.
await expectMonthlyCounter(organisation, 'documentCount', 1);
});
});
@@ -775,8 +782,9 @@ test.describe('Organisation dynamic rate limits & quotas', () => {
await expectNotGlobalLimited(res);
expect(res.ok(), `redistribute should succeed: ${await res.text()}`).toBeTruthy();
// Windowed pass with a null quota must NOT touch the monthly counter.
await expectMonthlyCounter(organisation, 'emailCount', 0);
// The windowed pass is now recorded even though the quota is null, so the
// counter advances by the batch size (recipientIds.length).
await expectMonthlyCounter(organisation, 'emailCount', recipientIds.length);
});
test('redistribute is blocked when recipient count exceeds the email window', async ({ request }) => {
packages/lib/server-only/rate-limit/check-monthly-quota.ts
+5 -4
View File
@@ -19,10 +19,6 @@ const COUNTER_COLUMN = {
} as const satisfies Record<LimitCounter, string>;
export const checkMonthlyQuota = async (opts: CheckMonthlyQuotaOptions): Promise<void> => {
if (opts.quota === null) {
return;
}
if (opts.quota === 0) {
throw new AppError(AppErrorCode.TOO_MANY_REQUESTS, {
message:
@@ -52,6 +48,11 @@ export const checkMonthlyQuota = async (opts: CheckMonthlyQuotaOptions): Promise
},
});
// For unlimited quotas, we still allow the request to send so we can collect the monthly stat.
if (opts.quota === null) {
return;
}
const newCount = latestMonthlyStat[column];
const previousCount = newCount - opts.count;