Skip to content

Commit 3a85ead

Browse files
committed
Improve Sentry error capture for Actions
1 parent 81c7d19 commit 3a85ead

10 files changed

Lines changed: 468 additions & 36 deletions

File tree

src/actions/newsletter/__tests__/action.spec.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,82 @@ describe('newsletter.subscribe.handler', () => {
300300

301301
expect(throwActionError).toHaveBeenCalledWith(
302302
expect.any(Error),
303-
{ route: '/_actions/newsletter/subscribe', operation: 'subscribe' }
303+
expect.objectContaining({
304+
route: '/_actions/newsletter/subscribe',
305+
operation: 'subscribe',
306+
extra: expect.objectContaining({
307+
stage: 'buildRequestFingerprint',
308+
source: 'newsletter_form',
309+
input: expect.objectContaining({
310+
consentGiven: true,
311+
emailDomain: 'example.com',
312+
hasDataSubjectId: false,
313+
hasFirstName: false,
314+
subjectIdSource: 'pending',
315+
}),
316+
}),
317+
})
318+
)
319+
})
320+
321+
it('logs handled action errors with subscribe stage context', async () => {
322+
vi.resetModules()
323+
324+
const { createPendingSubscription } = await import('@actions/newsletter/domain')
325+
vi.mocked(createPendingSubscription).mockRejectedValueOnce(
326+
new (await import('@actions/utils/errors')).ActionsFunctionError('db unavailable', {
327+
status: 500,
328+
})
329+
)
330+
331+
const { newsletter } = await import('../action')
332+
const { handleActionsFunctionError } = await import('@actions/utils/errors')
333+
334+
const context = {
335+
request: new Request('https://example.com/_actions/newsletter/subscribe', {
336+
method: 'POST',
337+
headers: { 'user-agent': 'ua-6' },
338+
}),
339+
cookies: {} as unknown,
340+
clientAddress: '203.0.113.16',
341+
}
342+
343+
await expect(
344+
getMockedHandler<NewsletterSubscribeInput, NewsletterSubscribeOutput>(newsletter.subscribe)(
345+
{ email: 'test@example.com', consentGiven: true },
346+
context
347+
)
348+
).rejects.toMatchObject({
349+
name: 'ActionsFunctionError',
350+
status: 500,
351+
message: 'db unavailable',
352+
})
353+
354+
expect(handleActionsFunctionError).toHaveBeenCalledWith(
355+
expect.objectContaining({
356+
name: 'ActionsFunctionError',
357+
status: 500,
358+
}),
359+
expect.objectContaining({
360+
route: '/_actions/newsletter/subscribe',
361+
operation: 'subscribe',
362+
extra: expect.objectContaining({
363+
stage: 'createPendingSubscription',
364+
fingerprint: 'fingerprint-1',
365+
source: 'newsletter_form',
366+
input: expect.objectContaining({
367+
emailDomain: 'example.com',
368+
emailLength: 16,
369+
consentGiven: true,
370+
subjectIdSource: 'generated',
371+
}),
372+
request: expect.objectContaining({
373+
hasClientAddress: true,
374+
hasUserAgent: true,
375+
rateLimitIdentifier: 'newsletter:consent:fingerprint-1',
376+
}),
377+
}),
378+
})
304379
)
305380
})
306381
})

src/actions/newsletter/action.ts

Lines changed: 94 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,61 @@ const confirmSchema = z.object({
2626
token: z.string().min(1),
2727
})
2828

29+
type NewsletterSubscribeStage =
30+
| 'buildRequestFingerprint'
31+
| 'checkRateLimit'
32+
| 'validateEmail'
33+
| 'validateConsent'
34+
| 'resolveDataSubjectId'
35+
| 'createConsentRecord'
36+
| 'createPendingSubscription'
37+
| 'sendConfirmationEmail'
38+
39+
const getEmailDomain = (email: string): string | undefined => {
40+
const normalizedEmail = email.trim().toLowerCase()
41+
const atIndex = normalizedEmail.lastIndexOf('@')
42+
43+
if (atIndex === -1 || atIndex === normalizedEmail.length - 1) {
44+
return undefined
45+
}
46+
47+
return normalizedEmail.slice(atIndex + 1)
48+
}
49+
50+
const buildSubscribeErrorExtra = (options: {
51+
body: z.infer<typeof subscribeSchema>
52+
fingerprint?: string
53+
consentFunctional: boolean
54+
stage: NewsletterSubscribeStage
55+
userAgent: string
56+
clientAddress?: string
57+
rateLimitIdentifier?: string
58+
subjectIdSource: 'generated' | 'provided' | 'pending'
59+
}): Record<string, unknown> => {
60+
return {
61+
stage: options.stage,
62+
source: 'newsletter_form',
63+
consentFunctional: options.consentFunctional,
64+
fingerprint: options.fingerprint,
65+
request: {
66+
hasClientAddress:
67+
typeof options.clientAddress === 'string' && options.clientAddress !== 'unknown',
68+
hasUserAgent: options.userAgent !== 'unknown',
69+
rateLimitIdentifier: options.rateLimitIdentifier,
70+
},
71+
input: {
72+
emailDomain: getEmailDomain(options.body.email),
73+
emailLength: options.body.email.trim().length,
74+
consentGiven: Boolean(options.body.consentGiven),
75+
hasFirstName:
76+
typeof options.body.firstName === 'string' && options.body.firstName.trim().length > 0,
77+
hasDataSubjectId:
78+
typeof options.body.DataSubjectId === 'string' && options.body.DataSubjectId.length > 0,
79+
subjectIdSource: options.subjectIdSource,
80+
},
81+
}
82+
}
83+
2984
export const newsletter = {
3085
subscribe: defineAction({
3186
accept: 'json',
@@ -35,16 +90,26 @@ export const newsletter = {
3590
context
3691
): Promise<{ success: true; message: string; requiresConfirmation: true }> => {
3792
const route = '/_actions/newsletter/subscribe'
93+
let stage: NewsletterSubscribeStage = 'buildRequestFingerprint'
94+
let fingerprint: string | undefined
95+
let consentFunctional = false
96+
let rateLimitIdentifier: string | undefined
97+
let subjectIdSource: 'generated' | 'provided' | 'pending' = 'pending'
98+
const userAgent = context.request.headers.get('user-agent') || 'unknown'
3899

39100
try {
40-
const { fingerprint } = buildRequestFingerprint({
101+
const requestFingerprint = buildRequestFingerprint({
41102
route,
42103
request: context.request,
43104
cookies: context.cookies,
44105
clientAddress: context.clientAddress,
45106
})
46107

47-
const rateLimitIdentifier = createRateLimitIdentifier('newsletter:consent', fingerprint)
108+
fingerprint = requestFingerprint.fingerprint
109+
consentFunctional = requestFingerprint.consentFunctional
110+
111+
stage = 'checkRateLimit'
112+
rateLimitIdentifier = createRateLimitIdentifier('newsletter:consent', fingerprint)
48113
const { success, reset } = await checkRateLimit(rateLimiters.consent, rateLimitIdentifier)
49114

50115
if (!success) {
@@ -53,24 +118,29 @@ export const newsletter = {
53118
throw new ActionsFunctionError(`Try again in ${retryAfterSeconds}s`, { status: 429 })
54119
}
55120

121+
stage = 'validateEmail'
56122
const validatedEmail = validateEmail(body.email)
57123

124+
stage = 'validateConsent'
58125
if (!body.consentGiven) {
59126
throw new ActionsFunctionError(
60127
'You must consent to receive marketing emails to subscribe.',
61128
{ status: 400 }
62129
)
63130
}
64131

65-
const userAgent = context.request.headers.get('user-agent') || 'unknown'
66-
132+
stage = 'resolveDataSubjectId'
67133
let subjectId = body.DataSubjectId
68134
if (!subjectId) {
69135
subjectId = uuidv4()
136+
subjectIdSource = 'generated'
70137
} else if (!uuidValidate(subjectId)) {
71138
throw new ActionsFunctionError('Invalid DataSubjectId format', { status: 400 })
139+
} else {
140+
subjectIdSource = 'provided'
72141
}
73142

143+
stage = 'createConsentRecord'
74144
await createConsentRecord({
75145
dataSubjectId: subjectId,
76146
email: validatedEmail,
@@ -86,6 +156,7 @@ export const newsletter = {
86156
verified: false,
87157
})
88158

159+
stage = 'createPendingSubscription'
89160
const token = await createPendingSubscription({
90161
email: validatedEmail,
91162
...(body.firstName && { firstName: body.firstName }),
@@ -96,6 +167,7 @@ export const newsletter = {
96167
source: 'newsletter_form',
97168
})
98169

170+
stage = 'sendConfirmationEmail'
99171
await sendConfirmationEmail(validatedEmail, token, body.firstName)
100172

101173
return {
@@ -104,10 +176,27 @@ export const newsletter = {
104176
requiresConfirmation: true,
105177
}
106178
} catch (error) {
179+
const errorContext = {
180+
route,
181+
operation: 'subscribe',
182+
extra: buildSubscribeErrorExtra({
183+
body,
184+
fingerprint,
185+
consentFunctional,
186+
stage,
187+
userAgent,
188+
clientAddress: context.clientAddress,
189+
rateLimitIdentifier,
190+
subjectIdSource,
191+
}),
192+
} as const
193+
107194
if (error instanceof ActionsFunctionError) {
195+
handleActionsFunctionError(error, errorContext)
108196
throw error
109197
}
110-
throwActionError(error, { route, operation: 'subscribe' })
198+
199+
throwActionError(error, errorContext)
111200
}
112201
},
113202
}),

src/actions/utils/errors/__tests__/actionsFunctionHandler.spec.ts

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,16 @@
1-
import { describe, expect, it, vi } from 'vitest'
1+
import { beforeEach, describe, expect, it, vi } from 'vitest'
2+
3+
const captureExceptionMock = vi.fn()
4+
const setTagsMock = vi.fn()
5+
const setTagMock = vi.fn()
6+
const setExtrasMock = vi.fn()
7+
const withScopeMock = vi.fn((callback: (_scope: unknown) => void) => {
8+
callback({
9+
setTags: setTagsMock,
10+
setTag: setTagMock,
11+
setExtras: setExtrasMock,
12+
})
13+
})
214

315
vi.mock('astro:actions', () => {
416
class ActionError extends Error {
@@ -26,10 +38,18 @@ vi.mock('@actions/utils/sentry', () => ({
2638
}))
2739

2840
vi.mock('@sentry/astro', () => ({
29-
captureException: () => undefined,
30-
withScope: (_fn: (_scope: unknown) => void) => undefined,
41+
captureException: captureExceptionMock,
42+
withScope: withScopeMock,
3143
}))
3244

45+
beforeEach(() => {
46+
captureExceptionMock.mockReset()
47+
setTagsMock.mockReset()
48+
setTagMock.mockReset()
49+
setExtrasMock.mockReset()
50+
withScopeMock.mockClear()
51+
})
52+
3353
describe('actionsFunctionHandler', () => {
3454
it('converts server errors into ActionError with fallback message', async () => {
3555
const { ActionsFunctionError } = await import('../ActionsFunctionError')
@@ -64,4 +84,71 @@ describe('actionsFunctionHandler', () => {
6484
expect(normalized.status).toBe(500)
6585
expect(normalized.message).toBe('boom')
6686
})
87+
88+
it('includes merged details in structured logs', async () => {
89+
const { ActionsFunctionError } = await import('../ActionsFunctionError')
90+
const { formatActionsErrorLogEntry } = await import('../actionsFunctionHandler')
91+
92+
const error = new ActionsFunctionError('DB exploded', {
93+
status: 500,
94+
route: 'actions:test',
95+
appCode: 'DB_WRITE_FAILED',
96+
details: { stage: 'createPendingSubscription' },
97+
})
98+
99+
const entry = formatActionsErrorLogEntry(error, {
100+
route: 'actions:test',
101+
operation: 'subscribe',
102+
extra: { fingerprint: 'fingerprint-1' },
103+
})
104+
105+
expect(entry.appCode).toBe('DB_WRITE_FAILED')
106+
expect(entry.details).toEqual({
107+
stage: 'createPendingSubscription',
108+
fingerprint: 'fingerprint-1',
109+
})
110+
})
111+
112+
it('forwards merged details and appCode to sentry in production', async () => {
113+
vi.resetModules()
114+
115+
vi.doMock('@actions/utils/environment/environmentActions', () => ({
116+
isDev: () => false,
117+
isProd: () => true,
118+
isTest: () => false,
119+
isUnitTest: () => false,
120+
}))
121+
122+
const { ActionsFunctionError } = await import('../ActionsFunctionError')
123+
const { handleActionsFunctionError } = await import('../actionsFunctionHandler')
124+
125+
const error = new ActionsFunctionError('DB exploded', {
126+
status: 500,
127+
route: 'actions:test',
128+
appCode: 'DB_WRITE_FAILED',
129+
details: { stage: 'persist' },
130+
})
131+
132+
const normalized = handleActionsFunctionError(error, {
133+
route: 'actions:test',
134+
operation: 'subscribe',
135+
extra: { fingerprint: 'fingerprint-1' },
136+
})
137+
138+
expect(normalized).toBeInstanceOf(ActionsFunctionError)
139+
expect(setTagsMock).toHaveBeenCalledWith(
140+
expect.objectContaining({
141+
route: 'actions:test',
142+
status: '500',
143+
retryable: 'true',
144+
appCode: 'DB_WRITE_FAILED',
145+
operation: 'subscribe',
146+
})
147+
)
148+
expect(setExtrasMock).toHaveBeenCalledWith({
149+
stage: 'persist',
150+
fingerprint: 'fingerprint-1',
151+
})
152+
expect(captureExceptionMock).toHaveBeenCalledWith(expect.any(ActionsFunctionError))
153+
})
67154
})

0 commit comments

Comments
 (0)