Skip to content

Commit 077ee24

Browse files
committed
improvement(billing): self-heal null usage limits and debounce api-key last-used writes
1 parent e2523e0 commit 077ee24

4 files changed

Lines changed: 212 additions & 8 deletions

File tree

apps/sim/lib/api-key/service.test.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({
5252
}))
5353

5454
import { hashApiKey } from '@/lib/api-key/crypto'
55-
import { authenticateApiKeyFromHeader } from '@/lib/api-key/service'
55+
import { authenticateApiKeyFromHeader, updateApiKeyLastUsed } from '@/lib/api-key/service'
5656

5757
function personalKeyRecord(overrides: Partial<Record<string, unknown>> = {}) {
5858
return {
@@ -141,3 +141,33 @@ describe('authenticateApiKeyFromHeader', () => {
141141
expect(JSON.stringify(filter)).toContain(expected)
142142
})
143143
})
144+
145+
describe('updateApiKeyLastUsed', () => {
146+
beforeEach(() => {
147+
vi.clearAllMocks()
148+
})
149+
150+
it('only writes when the stored lastUsed is missing or stale', async () => {
151+
await updateApiKeyLastUsed('key-1')
152+
153+
expect(dbChainMockFns.update).toHaveBeenCalledTimes(1)
154+
expect(dbChainMockFns.set).toHaveBeenCalledWith({ lastUsed: expect.any(Date) })
155+
const [condition] = dbChainMockFns.where.mock.calls[0]
156+
expect(condition).toMatchObject({
157+
type: 'and',
158+
conditions: [
159+
{ type: 'eq', right: 'key-1' },
160+
{ type: 'or', conditions: [{ type: 'isNull' }, { type: 'lt' }] },
161+
],
162+
})
163+
})
164+
165+
it('swallows database errors instead of failing the request', async () => {
166+
dbChainMockFns.update.mockImplementationOnce(() => {
167+
throw new Error('connection lost')
168+
})
169+
170+
await expect(updateApiKeyLastUsed('key-1')).resolves.toBeUndefined()
171+
expect(serviceLogger.error).toHaveBeenCalled()
172+
})
173+
})

apps/sim/lib/api-key/service.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { db } from '@sim/db'
22
import { apiKey as apiKeyTable, user as userTable } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
4-
import { and, eq } from 'drizzle-orm'
4+
import { and, eq, isNull, lt, or } from 'drizzle-orm'
55
import { hashApiKey } from '@/lib/api-key/crypto'
66
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
77
import { getWorkspaceBillingSettings, type WorkspaceBillingSettings } from '@/lib/workspaces/utils'
@@ -136,12 +136,28 @@ export async function authenticateApiKeyFromHeader(
136136
}
137137
}
138138

139+
const LAST_USED_DEBOUNCE_MS = 60_000
140+
139141
/**
140-
* Update the last used timestamp for an API key
142+
* Update the last used timestamp for an API key.
143+
*
144+
* `lastUsed` is display-only, so the write is debounced: it only fires when
145+
* the stored value is older than {@link LAST_USED_DEBOUNCE_MS}. High-traffic
146+
* keys otherwise rewrite the same row on every request, serializing
147+
* concurrent requests behind row locks.
141148
*/
142149
export async function updateApiKeyLastUsed(keyId: string): Promise<void> {
143150
try {
144-
await db.update(apiKeyTable).set({ lastUsed: new Date() }).where(eq(apiKeyTable.id, keyId))
151+
const staleBefore = new Date(Date.now() - LAST_USED_DEBOUNCE_MS)
152+
await db
153+
.update(apiKeyTable)
154+
.set({ lastUsed: new Date() })
155+
.where(
156+
and(
157+
eq(apiKeyTable.id, keyId),
158+
or(isNull(apiKeyTable.lastUsed), lt(apiKeyTable.lastUsed, staleBefore))
159+
)
160+
)
145161
} catch (error) {
146162
logger.error('Error updating API key last used:', error)
147163
}
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/**
2+
* Tests for getUserUsageLimit.
3+
*
4+
* Org-scoped members carry a null `currentUsageLimit` by design, so a user
5+
* whose subscription stops being org-scoped without a resync is left null.
6+
* The limit read must self-heal that state to the plan default instead of
7+
* failing closed and blocking every execution.
8+
*
9+
* @vitest-environment node
10+
*/
11+
import { dbChainMock, dbChainMockFns } from '@sim/testing'
12+
import { beforeEach, describe, expect, it, vi } from 'vitest'
13+
14+
vi.mock('@sim/db', () => dbChainMock)
15+
16+
const {
17+
mockGetFreeTierLimit,
18+
mockGetPerUserMinimumLimit,
19+
mockHasPaidSubscriptionStatus,
20+
mockIsOrgScopedSubscription,
21+
} = vi.hoisted(() => ({
22+
mockGetFreeTierLimit: vi.fn(),
23+
mockGetPerUserMinimumLimit: vi.fn(),
24+
mockHasPaidSubscriptionStatus: vi.fn(),
25+
mockIsOrgScopedSubscription: vi.fn(),
26+
}))
27+
28+
vi.mock('@/lib/billing/subscriptions/utils', () => ({
29+
canEditUsageLimit: vi.fn(),
30+
getFreeTierLimit: mockGetFreeTierLimit,
31+
getPerUserMinimumLimit: mockGetPerUserMinimumLimit,
32+
getPlanPricing: vi.fn(() => ({ basePrice: 20 })),
33+
hasPaidSubscriptionStatus: mockHasPaidSubscriptionStatus,
34+
hasUsableSubscriptionAccess: vi.fn(),
35+
isOrgScopedSubscription: mockIsOrgScopedSubscription,
36+
}))
37+
38+
vi.mock('@/lib/billing/core/plan', () => ({
39+
getHighestPrioritySubscription: vi.fn(),
40+
}))
41+
42+
vi.mock('@/lib/billing/core/access', () => ({
43+
getEffectiveBillingStatus: vi.fn(),
44+
}))
45+
46+
vi.mock('@/lib/billing/core/usage-log', () => ({
47+
getBillingPeriodUsageCost: vi.fn(),
48+
}))
49+
50+
vi.mock('@/lib/billing/credits/daily-refresh', () => ({
51+
computeDailyRefreshConsumed: vi.fn(),
52+
getOrgMemberRefreshBounds: vi.fn(),
53+
}))
54+
55+
vi.mock('@/components/emails', () => ({
56+
getEmailSubject: vi.fn(),
57+
renderCreditsExhaustedEmail: vi.fn(),
58+
renderFreeTierUpgradeEmail: vi.fn(),
59+
renderUsageThresholdEmail: vi.fn(),
60+
}))
61+
62+
vi.mock('@/lib/messaging/email/mailer', () => ({
63+
sendEmail: vi.fn(),
64+
}))
65+
66+
vi.mock('@/lib/messaging/email/unsubscribe', () => ({
67+
getEmailPreferences: vi.fn(),
68+
}))
69+
70+
import { getUserUsageLimit } from '@/lib/billing/core/usage'
71+
72+
const PRO_SUBSCRIPTION = {
73+
id: 'sub-1',
74+
plan: 'pro',
75+
status: 'active',
76+
referenceId: 'user-1',
77+
seats: 1,
78+
periodStart: null,
79+
periodEnd: null,
80+
} as never
81+
82+
describe('getUserUsageLimit', () => {
83+
beforeEach(() => {
84+
vi.clearAllMocks()
85+
mockIsOrgScopedSubscription.mockReturnValue(false)
86+
})
87+
88+
it('returns the stored limit when set', async () => {
89+
dbChainMockFns.limit.mockResolvedValueOnce([{ currentUsageLimit: '25' }])
90+
91+
const limit = await getUserUsageLimit('user-1', null)
92+
93+
expect(limit).toBe(25)
94+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
95+
})
96+
97+
it('throws when no userStats row exists', async () => {
98+
dbChainMockFns.limit.mockResolvedValueOnce([])
99+
100+
await expect(getUserUsageLimit('user-1', null)).rejects.toThrow('No user stats record found')
101+
})
102+
103+
it('heals a null limit to the free-tier default for free users', async () => {
104+
dbChainMockFns.limit.mockResolvedValueOnce([{ currentUsageLimit: null }])
105+
mockGetFreeTierLimit.mockReturnValue(10)
106+
107+
const limit = await getUserUsageLimit('user-1', null)
108+
109+
expect(limit).toBe(10)
110+
expect(dbChainMockFns.update).toHaveBeenCalledTimes(1)
111+
expect(dbChainMockFns.set).toHaveBeenCalledWith({
112+
currentUsageLimit: '10',
113+
usageLimitUpdatedAt: expect.any(Date),
114+
})
115+
const [condition] = dbChainMockFns.where.mock.calls.at(-1) ?? []
116+
expect(condition).toMatchObject({
117+
type: 'and',
118+
conditions: [{ type: 'eq', right: 'user-1' }, { type: 'isNull' }],
119+
})
120+
})
121+
122+
it('heals a null limit to the plan minimum for paid personal subscriptions', async () => {
123+
dbChainMockFns.limit.mockResolvedValueOnce([{ currentUsageLimit: null }])
124+
mockHasPaidSubscriptionStatus.mockReturnValue(true)
125+
mockGetPerUserMinimumLimit.mockReturnValue(40)
126+
127+
const limit = await getUserUsageLimit('user-1', PRO_SUBSCRIPTION)
128+
129+
expect(limit).toBe(40)
130+
expect(mockGetPerUserMinimumLimit).toHaveBeenCalledWith(PRO_SUBSCRIPTION)
131+
expect(dbChainMockFns.set).toHaveBeenCalledWith({
132+
currentUsageLimit: '40',
133+
usageLimitUpdatedAt: expect.any(Date),
134+
})
135+
})
136+
})

apps/sim/lib/billing/core/usage.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { db } from '@sim/db'
22
import { member, organization, settings, user, userStats } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { generateId } from '@sim/utils/id'
5-
import { eq } from 'drizzle-orm'
5+
import { and, eq, isNull } from 'drizzle-orm'
66
import {
77
getEmailSubject,
88
renderCreditsExhaustedEmail,
@@ -495,6 +495,11 @@ export async function updateUserUsageLimit(
495495
* Get usage limit for a user (used by checkUsageStatus for server-side
496496
* checks). Org-scoped subs return the organization limit;
497497
* personally-scoped subs return the individual user limit from userStats.
498+
*
499+
* Org-scoped members carry a null `currentUsageLimit` by design (see
500+
* `syncUsageLimitsFromSubscription`). A user whose subscription stops being
501+
* org-scoped without a resync would otherwise stay null and fail closed on
502+
* every execution, so a null limit self-heals to the plan default here.
498503
*/
499504
export async function getUserUsageLimit(
500505
userId: string,
@@ -537,9 +542,26 @@ export async function getUserUsageLimit(
537542
}
538543

539544
if (!userStatsQuery[0].currentUsageLimit) {
540-
throw new Error(
541-
`Invalid null usage limit for ${subscription?.plan || 'free'} user: ${userId}. User stats must be properly initialized.`
542-
)
545+
const fallbackLimit =
546+
subscription && hasPaidSubscriptionStatus(subscription.status)
547+
? getPerUserMinimumLimit(subscription)
548+
: getFreeTierLimit()
549+
550+
await db
551+
.update(userStats)
552+
.set({
553+
currentUsageLimit: fallbackLimit.toString(),
554+
usageLimitUpdatedAt: new Date(),
555+
})
556+
.where(and(eq(userStats.userId, userId), isNull(userStats.currentUsageLimit)))
557+
558+
logger.warn('Healed null usage limit to plan default', {
559+
userId,
560+
plan: subscription?.plan || 'free',
561+
fallbackLimit,
562+
})
563+
564+
return fallbackLimit
543565
}
544566

545567
return toNumber(toDecimal(userStatsQuery[0].currentUsageLimit))

0 commit comments

Comments
 (0)