Skip to content

Commit 8caf794

Browse files
chargomechargome
authored
feat(react-router): Automatically flush on serverless for loaders/actions (#17234)
1 parent 12ac49a commit 8caf794

5 files changed

Lines changed: 140 additions & 51 deletions

File tree

‎packages/react-router/src/server/wrapServerAction.ts‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { SEMATTRS_HTTP_TARGET } from '@opentelemetry/semantic-conventions';
22
import type { SpanAttributes } from '@sentry/core';
33
import {
4+
flushIfServerless,
45
getActiveSpan,
56
getRootSpan,
67
SEMANTIC_ATTRIBUTE_SENTRY_OP,
@@ -59,17 +60,21 @@ export function wrapServerAction<T>(options: SpanOptions = {}, actionFn: (args:
5960
}
6061
}
6162

62-
return startSpan(
63-
{
64-
name,
65-
...options,
66-
attributes: {
67-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react-router.action',
68-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.action',
69-
...options.attributes,
63+
try {
64+
return await startSpan(
65+
{
66+
name,
67+
...options,
68+
attributes: {
69+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react-router.action',
70+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.action',
71+
...options.attributes,
72+
},
7073
},
71-
},
72-
() => actionFn(args),
73-
);
74+
() => actionFn(args),
75+
);
76+
} finally {
77+
await flushIfServerless();
78+
}
7479
};
7580
}

‎packages/react-router/src/server/wrapServerLoader.ts‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { SEMATTRS_HTTP_TARGET } from '@opentelemetry/semantic-conventions';
22
import type { SpanAttributes } from '@sentry/core';
33
import {
4+
flushIfServerless,
45
getActiveSpan,
56
getRootSpan,
67
SEMANTIC_ATTRIBUTE_SENTRY_OP,
@@ -59,17 +60,21 @@ export function wrapServerLoader<T>(options: SpanOptions = {}, loaderFn: (args:
5960
}
6061
}
6162
}
62-
return startSpan(
63-
{
64-
name,
65-
...options,
66-
attributes: {
67-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react-router.loader',
68-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.loader',
69-
...options.attributes,
63+
try {
64+
return await startSpan(
65+
{
66+
name,
67+
...options,
68+
attributes: {
69+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.react-router.loader',
70+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.react-router.loader',
71+
...options.attributes,
72+
},
7073
},
71-
},
72-
() => loaderFn(args),
73-
);
74+
() => loaderFn(args),
75+
);
76+
} finally {
77+
await flushIfServerless();
78+
}
7479
};
7580
}

‎packages/react-router/test/server/wrapSentryHandleRequest.test.ts‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -176,27 +176,6 @@ describe('wrapSentryHandleRequest', () => {
176176
});
177177
});
178178

179-
test('should not set span attributes when parameterized path does not exist', async () => {
180-
const mockActiveSpan = {};
181-
const mockRootSpan = { setAttributes: vi.fn() };
182-
183-
(getActiveSpan as unknown as ReturnType<typeof vi.fn>).mockReturnValue(mockActiveSpan);
184-
(getRootSpan as unknown as ReturnType<typeof vi.fn>).mockReturnValue(mockRootSpan);
185-
186-
const originalHandler = vi.fn().mockResolvedValue('test');
187-
const wrappedHandler = wrapSentryHandleRequest(originalHandler);
188-
189-
const routerContext = {
190-
staticHandlerContext: {
191-
matches: [],
192-
},
193-
} as any;
194-
195-
await wrappedHandler(new Request('https://guapo.chulo'), 200, new Headers(), routerContext, {} as any);
196-
197-
expect(mockRootSpan.setAttributes).not.toHaveBeenCalled();
198-
});
199-
200179
describe('getMetaTagTransformer', () => {
201180
beforeEach(() => {
202181
vi.clearAllMocks();

‎packages/react-router/test/server/wrapServerAction.test.ts‎

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,15 @@ import type { ActionFunctionArgs } from 'react-router';
33
import { beforeEach, describe, expect, it, vi } from 'vitest';
44
import { wrapServerAction } from '../../src/server/wrapServerAction';
55

6+
vi.mock('@sentry/core', async () => {
7+
const actual = await vi.importActual('@sentry/core');
8+
return {
9+
...actual,
10+
startSpan: vi.fn(),
11+
flushIfServerless: vi.fn(),
12+
};
13+
});
14+
615
describe('wrapServerAction', () => {
716
beforeEach(() => {
817
vi.clearAllMocks();
@@ -12,11 +21,12 @@ describe('wrapServerAction', () => {
1221
const mockActionFn = vi.fn().mockResolvedValue('result');
1322
const mockArgs = { request: new Request('http://test.com') } as ActionFunctionArgs;
1423

15-
const spy = vi.spyOn(core, 'startSpan');
24+
(core.startSpan as any).mockImplementation((_: any, fn: any) => fn());
25+
1626
const wrappedAction = wrapServerAction({}, mockActionFn);
1727
await wrappedAction(mockArgs);
1828

19-
expect(spy).toHaveBeenCalledWith(
29+
expect(core.startSpan).toHaveBeenCalledWith(
2030
{
2131
name: 'Executing Server Action',
2232
attributes: {
@@ -27,6 +37,7 @@ describe('wrapServerAction', () => {
2737
expect.any(Function),
2838
);
2939
expect(mockActionFn).toHaveBeenCalledWith(mockArgs);
40+
expect(core.flushIfServerless).toHaveBeenCalled();
3041
});
3142

3243
it('should wrap an action function with custom options', async () => {
@@ -40,11 +51,12 @@ describe('wrapServerAction', () => {
4051
const mockActionFn = vi.fn().mockResolvedValue('result');
4152
const mockArgs = { request: new Request('http://test.com') } as ActionFunctionArgs;
4253

43-
const spy = vi.spyOn(core, 'startSpan');
54+
(core.startSpan as any).mockImplementation((_: any, fn: any) => fn());
55+
4456
const wrappedAction = wrapServerAction(customOptions, mockActionFn);
4557
await wrappedAction(mockArgs);
4658

47-
expect(spy).toHaveBeenCalledWith(
59+
expect(core.startSpan).toHaveBeenCalledWith(
4860
{
4961
name: 'Custom Action',
5062
attributes: {
@@ -56,5 +68,43 @@ describe('wrapServerAction', () => {
5668
expect.any(Function),
5769
);
5870
expect(mockActionFn).toHaveBeenCalledWith(mockArgs);
71+
expect(core.flushIfServerless).toHaveBeenCalled();
72+
});
73+
74+
it('should call flushIfServerless on successful execution', async () => {
75+
const mockActionFn = vi.fn().mockResolvedValue('result');
76+
const mockArgs = { request: new Request('http://test.com') } as ActionFunctionArgs;
77+
78+
(core.startSpan as any).mockImplementation((_: any, fn: any) => fn());
79+
80+
const wrappedAction = wrapServerAction({}, mockActionFn);
81+
await wrappedAction(mockArgs);
82+
83+
expect(core.flushIfServerless).toHaveBeenCalled();
84+
});
85+
86+
it('should call flushIfServerless even when action throws an error', async () => {
87+
const mockError = new Error('Action failed');
88+
const mockActionFn = vi.fn().mockRejectedValue(mockError);
89+
const mockArgs = { request: new Request('http://test.com') } as ActionFunctionArgs;
90+
91+
(core.startSpan as any).mockImplementation((_: any, fn: any) => fn());
92+
93+
const wrappedAction = wrapServerAction({}, mockActionFn);
94+
95+
await expect(wrappedAction(mockArgs)).rejects.toThrow('Action failed');
96+
expect(core.flushIfServerless).toHaveBeenCalled();
97+
});
98+
99+
it('should propagate errors from action function', async () => {
100+
const mockError = new Error('Test error');
101+
const mockActionFn = vi.fn().mockRejectedValue(mockError);
102+
const mockArgs = { request: new Request('http://test.com') } as ActionFunctionArgs;
103+
104+
(core.startSpan as any).mockImplementation((_: any, fn: any) => fn());
105+
106+
const wrappedAction = wrapServerAction({}, mockActionFn);
107+
108+
await expect(wrappedAction(mockArgs)).rejects.toBe(mockError);
59109
});
60110
});

‎packages/react-router/test/server/wrapServerLoader.test.ts‎

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,15 @@ import type { LoaderFunctionArgs } from 'react-router';
33
import { beforeEach, describe, expect, it, vi } from 'vitest';
44
import { wrapServerLoader } from '../../src/server/wrapServerLoader';
55

6+
vi.mock('@sentry/core', async () => {
7+
const actual = await vi.importActual('@sentry/core');
8+
return {
9+
...actual,
10+
startSpan: vi.fn(),
11+
flushIfServerless: vi.fn(),
12+
};
13+
});
14+
615
describe('wrapServerLoader', () => {
716
beforeEach(() => {
817
vi.clearAllMocks();
@@ -12,11 +21,12 @@ describe('wrapServerLoader', () => {
1221
const mockLoaderFn = vi.fn().mockResolvedValue('result');
1322
const mockArgs = { request: new Request('http://test.com') } as LoaderFunctionArgs;
1423

15-
const spy = vi.spyOn(core, 'startSpan');
24+
(core.startSpan as any).mockImplementation((_: any, fn: any) => fn());
25+
1626
const wrappedLoader = wrapServerLoader({}, mockLoaderFn);
1727
await wrappedLoader(mockArgs);
1828

19-
expect(spy).toHaveBeenCalledWith(
29+
expect(core.startSpan).toHaveBeenCalledWith(
2030
{
2131
name: 'Executing Server Loader',
2232
attributes: {
@@ -27,6 +37,7 @@ describe('wrapServerLoader', () => {
2737
expect.any(Function),
2838
);
2939
expect(mockLoaderFn).toHaveBeenCalledWith(mockArgs);
40+
expect(core.flushIfServerless).toHaveBeenCalled();
3041
});
3142

3243
it('should wrap a loader function with custom options', async () => {
@@ -40,11 +51,12 @@ describe('wrapServerLoader', () => {
4051
const mockLoaderFn = vi.fn().mockResolvedValue('result');
4152
const mockArgs = { request: new Request('http://test.com') } as LoaderFunctionArgs;
4253

43-
const spy = vi.spyOn(core, 'startSpan');
54+
(core.startSpan as any).mockImplementation((_: any, fn: any) => fn());
55+
4456
const wrappedLoader = wrapServerLoader(customOptions, mockLoaderFn);
4557
await wrappedLoader(mockArgs);
4658

47-
expect(spy).toHaveBeenCalledWith(
59+
expect(core.startSpan).toHaveBeenCalledWith(
4860
{
4961
name: 'Custom Loader',
5062
attributes: {
@@ -56,5 +68,43 @@ describe('wrapServerLoader', () => {
5668
expect.any(Function),
5769
);
5870
expect(mockLoaderFn).toHaveBeenCalledWith(mockArgs);
71+
expect(core.flushIfServerless).toHaveBeenCalled();
72+
});
73+
74+
it('should call flushIfServerless on successful execution', async () => {
75+
const mockLoaderFn = vi.fn().mockResolvedValue('result');
76+
const mockArgs = { request: new Request('http://test.com') } as LoaderFunctionArgs;
77+
78+
(core.startSpan as any).mockImplementation((_: any, fn: any) => fn());
79+
80+
const wrappedLoader = wrapServerLoader({}, mockLoaderFn);
81+
await wrappedLoader(mockArgs);
82+
83+
expect(core.flushIfServerless).toHaveBeenCalled();
84+
});
85+
86+
it('should call flushIfServerless even when loader throws an error', async () => {
87+
const mockError = new Error('Loader failed');
88+
const mockLoaderFn = vi.fn().mockRejectedValue(mockError);
89+
const mockArgs = { request: new Request('http://test.com') } as LoaderFunctionArgs;
90+
91+
(core.startSpan as any).mockImplementation((_: any, fn: any) => fn());
92+
93+
const wrappedLoader = wrapServerLoader({}, mockLoaderFn);
94+
95+
await expect(wrappedLoader(mockArgs)).rejects.toThrow('Loader failed');
96+
expect(core.flushIfServerless).toHaveBeenCalled();
97+
});
98+
99+
it('should propagate errors from loader function', async () => {
100+
const mockError = new Error('Test error');
101+
const mockLoaderFn = vi.fn().mockRejectedValue(mockError);
102+
const mockArgs = { request: new Request('http://test.com') } as LoaderFunctionArgs;
103+
104+
(core.startSpan as any).mockImplementation((_: any, fn: any) => fn());
105+
106+
const wrappedLoader = wrapServerLoader({}, mockLoaderFn);
107+
108+
await expect(wrappedLoader(mockArgs)).rejects.toBe(mockError);
59109
});
60110
});

0 commit comments

Comments
 (0)