Skip to content

Commit 9192a07

Browse files
committed
WIP express example
1 parent a10178e commit 9192a07

File tree

6 files changed

+98
-96
lines changed

6 files changed

+98
-96
lines changed

packages/core/src/integrations/express/index.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,12 @@ const getExpressExport = (express: ExpressModuleExport): ExpressExport =>
6969
* import express from 'express';
7070
* import * as Sentry from '@sentry/deno'; // or any SDK that extends core
7171
*
72-
* Sentry.patchExpressModule({ express })
72+
* Sentry.patchExpressModule(express, () => ({}));
73+
* ```
7374
*/
74-
export const patchExpressModule = (options: ExpressIntegrationOptions) => {
75+
export const patchExpressModule = (moduleExports: ExpressModuleExport, getOptions: () => ExpressIntegrationOptions) => {
7576
// pass in the require() or import() result of express
76-
const express = getExpressExport(options.express);
77+
const express = getExpressExport(moduleExports);
7778
const routerProto: ExpressRouterv4 | ExpressRouterv5 | undefined = isExpressWithRouterPrototype(express)
7879
? express.Router.prototype // Express v5
7980
: isExpressWithoutRouterPrototype(express)
@@ -93,7 +94,7 @@ export const patchExpressModule = (options: ExpressIntegrationOptions) => {
9394
function routeTrace(this: ExpressRouter, ...args: Parameters<typeof originalRouteMethod>[]) {
9495
const route = originalRouteMethod.apply(this, args);
9596
const layer = this.stack[this.stack.length - 1] as ExpressLayer;
96-
patchLayer(options, layer, getLayerPath(args));
97+
patchLayer(getOptions, layer, getLayerPath(args));
9798
return route;
9899
},
99100
);
@@ -113,7 +114,7 @@ export const patchExpressModule = (options: ExpressIntegrationOptions) => {
113114
if (!layer) {
114115
return route;
115116
}
116-
patchLayer(options, layer, getLayerPath(args));
117+
patchLayer(getOptions, layer, getLayerPath(args));
117118
return route;
118119
},
119120
);
@@ -141,7 +142,7 @@ export const patchExpressModule = (options: ExpressIntegrationOptions) => {
141142
if (router) {
142143
const layer = router.stack[router.stack.length - 1];
143144
if (layer) {
144-
patchLayer(options, layer, getLayerPath(args));
145+
patchLayer(getOptions, layer, getLayerPath(args));
145146
}
146147
}
147148
return route;

packages/core/src/integrations/express/patch-layer.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ export type ExpressPatchLayerOptions = Pick<
6161
'onRouteResolved' | 'ignoreLayers' | 'ignoreLayersType'
6262
>;
6363

64-
export function patchLayer(options: ExpressPatchLayerOptions, maybeLayer?: ExpressLayer, layerPath?: string): void {
64+
export function patchLayer(
65+
getOptions: () => ExpressPatchLayerOptions,
66+
maybeLayer?: ExpressLayer,
67+
layerPath?: string,
68+
): void {
6569
if (!maybeLayer?.handle) {
6670
return;
6771
}
@@ -86,6 +90,8 @@ export function patchLayer(options: ExpressPatchLayerOptions, maybeLayer?: Expre
8690
//oxlint-disable-next-line no-explicit-any
8791
...otherArgs: any[]
8892
) {
93+
const options = getOptions();
94+
8995
// Set normalizedRequest here because expressRequestHandler middleware
9096
// (registered via setupExpressErrorHandler) is added after routes and
9197
// therefore never runs for successful requests — route handlers typically

packages/core/src/integrations/express/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ export type ExpressRouter = {
136136
export type IgnoreMatcher = string | RegExp | ((name: string) => boolean);
137137

138138
export type ExpressIntegrationOptions = {
139-
express: ExpressModuleExport; //Express
140139
/** Ignore specific based on their name */
141140
ignoreLayers?: IgnoreMatcher[];
142141
/** Ignore specific layers based on their type */

packages/core/test/lib/integrations/express/index.test.ts

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,12 @@ vi.mock('../../../../src/utils/debug-logger', () => ({
6161
}));
6262

6363
beforeEach(() => (patchLayerCalls.length = 0));
64-
const patchLayerCalls: [options: ExpressIntegrationOptions, layer: ExpressLayer, layerPath?: string][] = [];
64+
const patchLayerCalls: [getOptions: () => ExpressIntegrationOptions, layer: ExpressLayer, layerPath?: string][] = [];
6565

6666
vi.mock('../../../../src/integrations/express/patch-layer', () => ({
67-
patchLayer: (options: ExpressIntegrationOptions, layer?: ExpressLayer, layerPath?: string) => {
67+
patchLayer: (getOptions: () => ExpressIntegrationOptions, layer?: ExpressLayer, layerPath?: string) => {
6868
if (layer) {
69-
patchLayerCalls.push([options, layer, layerPath]);
69+
patchLayerCalls.push([getOptions, layer, layerPath]);
7070
}
7171
},
7272
}));
@@ -131,24 +131,22 @@ function getExpress5(): ExpressExportv5 & { spies: ExpressSpies } {
131131
describe('patchExpressModule', () => {
132132
it('throws trying to patch/unpatch the wrong thing', () => {
133133
expect(() => {
134-
patchExpressModule({
135-
express: {} as unknown as ExpressModuleExport,
136-
} as unknown as ExpressIntegrationOptions);
134+
patchExpressModule({} as unknown as ExpressModuleExport, () => ({}));
137135
}).toThrowError('no valid Express route function to instrument');
138136
});
139137

140138
it('can patch and restore expressv4 style module', () => {
141139
for (const useDefault of [false, true]) {
142140
const express = getExpress4();
143-
const module = useDefault ? { default: express } : express;
141+
const moduleExports = useDefault ? { default: express } : express;
144142
const r = express.Router as ExpressRouterv4;
145143
const a = express.application;
146-
const options = { express: module } as unknown as ExpressIntegrationOptions;
144+
const getOptions = () => ({});
147145
expect((r.use as WrappedFunction).__sentry_original__).toBe(undefined);
148146
expect((r.route as WrappedFunction).__sentry_original__).toBe(undefined);
149147
expect((a.use as WrappedFunction).__sentry_original__).toBe(undefined);
150148

151-
patchExpressModule(options);
149+
patchExpressModule(moduleExports, getOptions);
152150

153151
expect(typeof (r.use as WrappedFunction).__sentry_original__).toBe('function');
154152
expect(typeof (r.route as WrappedFunction).__sentry_original__).toBe('function');
@@ -161,13 +159,13 @@ describe('patchExpressModule', () => {
161159
const express = getExpress5();
162160
const r = express.Router as ExpressRouterv5;
163161
const a = express.application;
164-
const module = useDefault ? { default: express } : express;
165-
const options = { express: module } as unknown as ExpressIntegrationOptions;
162+
const moduleExports = useDefault ? { default: express } : express;
163+
const getOptions = () => ({});
166164
expect((r.prototype.use as WrappedFunction).__sentry_original__).toBe(undefined);
167165
expect((r.prototype.route as WrappedFunction).__sentry_original__).toBe(undefined);
168166
expect((a.use as WrappedFunction).__sentry_original__).toBe(undefined);
169167

170-
patchExpressModule(options);
168+
patchExpressModule(moduleExports, getOptions);
171169

172170
expect(typeof (r.prototype.use as WrappedFunction).__sentry_original__).toBe('function');
173171
expect(typeof (r.prototype.route as WrappedFunction).__sentry_original__).toBe('function');
@@ -178,27 +176,27 @@ describe('patchExpressModule', () => {
178176
it('calls patched and original Router.route', () => {
179177
const expressv4 = getExpress4();
180178
const { spies } = expressv4;
181-
const options = { express: expressv4 };
182-
patchExpressModule(options);
179+
const getOptions = () => ({});
180+
patchExpressModule(expressv4, getOptions);
183181
expressv4.Router.route('a');
184182
expect(spies.routerRoute).toHaveBeenCalledExactlyOnceWith('a');
185183
});
186184

187185
it('calls patched and original Router.use', () => {
188186
const expressv4 = getExpress4();
189187
const { spies } = expressv4;
190-
const options = { express: expressv4 };
191-
patchExpressModule(options);
188+
const getOptions = () => ({});
189+
patchExpressModule(expressv4, getOptions);
192190
expressv4.Router.use('a');
193-
expect(patchLayerCalls).toStrictEqual([[options, { name: 'layerFinal' }, 'a']]);
191+
expect(patchLayerCalls).toStrictEqual([[getOptions, { name: 'layerFinal' }, 'a']]);
194192
expect(spies.routerUse).toHaveBeenCalledExactlyOnceWith('a');
195193
});
196194

197195
it('skips patchLayer call in Router.use if no layer in the stack', () => {
198196
const expressv4 = getExpress4();
199197
const { spies } = expressv4;
200-
const options = { express: expressv4 };
201-
patchExpressModule(options);
198+
const getOptions = () => ({});
199+
patchExpressModule(expressv4, getOptions);
202200
const { stack } = expressv4.Router;
203201
expressv4.Router.stack = [];
204202
expressv4.Router.use('a');
@@ -210,28 +208,28 @@ describe('patchExpressModule', () => {
210208
it('calls patched and original application.use', () => {
211209
const expressv4 = getExpress4();
212210
const { spies } = expressv4;
213-
const options = { express: expressv4 };
214-
patchExpressModule(options);
211+
const getOptions = () => ({});
212+
patchExpressModule(expressv4, getOptions);
215213
expressv4.application.use('a');
216-
expect(patchLayerCalls).toStrictEqual([[options, { name: 'layerFinal' }, 'a']]);
214+
expect(patchLayerCalls).toStrictEqual([[getOptions, { name: 'layerFinal' }, 'a']]);
217215
expect(spies.appUse).toHaveBeenCalledExactlyOnceWith('a');
218216
});
219217

220218
it('calls patched and original application.use on express v5', () => {
221219
const expressv5 = getExpress5();
222220
const { spies } = expressv5;
223-
const options = { express: expressv5 };
224-
patchExpressModule(options);
221+
const getOptions = () => ({});
222+
patchExpressModule(expressv5, getOptions);
225223
expressv5.application.use('a');
226-
expect(patchLayerCalls).toStrictEqual([[options, { name: 'layerFinal' }, 'a']]);
224+
expect(patchLayerCalls).toStrictEqual([[getOptions, { name: 'layerFinal' }, 'a']]);
227225
expect(spies.appUse).toHaveBeenCalledExactlyOnceWith('a');
228226
});
229227

230228
it('skips patchLayer on application.use if no router found', () => {
231229
const expressv4 = getExpress4();
232230
const { spies } = expressv4;
233-
const options = { express: expressv4 };
234-
patchExpressModule(options);
231+
const getOptions = () => ({});
232+
patchExpressModule(expressv4, getOptions);
235233
const app = expressv4.application as {
236234
_router?: ExpressRoute;
237235
};
@@ -246,8 +244,9 @@ describe('patchExpressModule', () => {
246244

247245
it('debug error when patching fails', () => {
248246
const expressv5 = getExpress5();
249-
patchExpressModule({ express: expressv5 });
250-
patchExpressModule({ express: expressv5 });
247+
const getOptions = () => ({});
248+
patchExpressModule(expressv5, getOptions);
249+
patchExpressModule(expressv5, getOptions);
251250
expect(debugErrors).toStrictEqual([
252251
['Failed to patch express route method:', new Error('Attempting to wrap method route multiple times')],
253252
['Failed to patch express use method:', new Error('Attempting to wrap method use multiple times')],

packages/core/test/lib/integrations/express/patch-layer.test.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -150,12 +150,12 @@ describe('patchLayer', () => {
150150
describe('no-ops', () => {
151151
it('if layer is missing', () => {
152152
// mostly for coverage, verifying it doesn't throw or anything
153-
patchLayer({});
153+
patchLayer(() => ({}));
154154
});
155155

156156
it('if layer.handle is missing', () => {
157157
// mostly for coverage, verifying it doesn't throw or anything
158-
patchLayer({}, { handle: null } as unknown as ExpressLayer);
158+
patchLayer(() => ({}), { handle: null } as unknown as ExpressLayer);
159159
});
160160

161161
it('if layer already patched', () => {
@@ -166,7 +166,7 @@ describe('patchLayer', () => {
166166
const layer = {
167167
handle: wrapped,
168168
} as unknown as ExpressLayer;
169-
patchLayer({}, layer);
169+
patchLayer(() => ({}), layer);
170170
expect(layer.handle).toBe(wrapped);
171171
});
172172

@@ -177,7 +177,7 @@ describe('patchLayer', () => {
177177
const layer = {
178178
handle: original,
179179
} as unknown as ExpressLayer;
180-
patchLayer({}, layer);
180+
patchLayer(() => ({}), layer);
181181
expect(layer.handle).toBe(original);
182182
});
183183

@@ -188,7 +188,7 @@ describe('patchLayer', () => {
188188
const layer = {
189189
handle: original,
190190
} as unknown as ExpressLayer;
191-
patchLayer({}, layer);
191+
patchLayer(() => ({}), layer);
192192
expect(getOriginalFunction(layer.handle)).toBe(original);
193193
});
194194
});
@@ -212,7 +212,7 @@ describe('patchLayer', () => {
212212
storeLayer(req, '/:boo');
213213
storeLayer(req, '/:car');
214214

215-
patchLayer(options, layer);
215+
patchLayer(() => options, layer);
216216
layer.handle(req, res);
217217
expect(layerHandleOriginal).toHaveBeenCalledOnce();
218218

@@ -244,7 +244,7 @@ describe('patchLayer', () => {
244244
storeLayer(req, '/:boo');
245245
storeLayer(req, '/:car');
246246

247-
patchLayer(options, layer, '/layerPath');
247+
patchLayer(() => options, layer, '/layerPath');
248248
layer.handle(req, res);
249249
expect(onRouteResolved).toHaveBeenCalledExactlyOnceWith('/a/:boo/:car/layerPath');
250250
expect(layerHandleOriginal).toHaveBeenCalledOnce();
@@ -290,7 +290,7 @@ describe('patchLayer', () => {
290290
// 'router' → router, 'bound dispatch' → request_handler, other → middleware
291291
const layerName = type === 'router' ? 'router' : 'bound dispatch';
292292
const layer = { name: layerName, handle: layerHandleOriginal } as unknown as ExpressLayer;
293-
patchLayer(options, layer, '/c');
293+
patchLayer(() => options, layer, '/c');
294294

295295
// storeLayer('/c') happens inside the patched handle, before being popped
296296
// after handle returns, storedLayers should be back to ['/a', '/b']
@@ -327,7 +327,7 @@ describe('patchLayer', () => {
327327
storeLayer(req, '/:boo');
328328
storeLayer(req, '/:car');
329329

330-
patchLayer(options, layer, '/layerPath');
330+
patchLayer(() => options, layer, '/layerPath');
331331
expect(getOriginalFunction(layer.handle)).toBe(layerHandleOriginal);
332332
expect(layer.handle.x).toBe(true);
333333
layer.handle.x = false;
@@ -382,7 +382,7 @@ describe('patchLayer', () => {
382382
storeLayer(req, '/:boo');
383383
storeLayer(req, '/:car');
384384

385-
patchLayer(options, layer);
385+
patchLayer(() => options, layer);
386386
expect(getOriginalFunction(layer.handle)).toBe(layerHandleOriginal);
387387
warnings.length = 0;
388388
layer.handle(req, res);
@@ -441,7 +441,7 @@ describe('patchLayer', () => {
441441
storeLayer(req, '/a');
442442
storeLayer(req, '/b');
443443

444-
patchLayer(options, layer, '/c');
444+
patchLayer(() => options, layer, '/c');
445445
layer.handle(req, res);
446446
expect(onRouteResolved).toHaveBeenCalledExactlyOnceWith('/a/b/c');
447447
const span = mockSpans[0];
@@ -482,7 +482,7 @@ describe('patchLayer', () => {
482482
storeLayer(req, '/a');
483483
storeLayer(req, '/b');
484484

485-
patchLayer(options, layer, '/c');
485+
patchLayer(() => options, layer, '/c');
486486
layer.handle(req, res);
487487
expect(onRouteResolved).toHaveBeenCalledExactlyOnceWith(undefined);
488488
const span = mockSpans[0];
@@ -526,7 +526,7 @@ describe('patchLayer', () => {
526526

527527
storeLayer(req, '/a');
528528
storeLayer(req, '/b');
529-
patchLayer(options, layer, '/c');
529+
patchLayer(() => options, layer, '/c');
530530

531531
expect(getStoredLayers(req)).toStrictEqual(['/a', '/b']);
532532
const callback = vi.fn(() => {
@@ -576,7 +576,7 @@ describe('patchLayer', () => {
576576

577577
storeLayer(req, '/a');
578578
storeLayer(req, '/b');
579-
patchLayer(options, layer, '/c');
579+
patchLayer(() => options, layer, '/c');
580580

581581
expect(getStoredLayers(req)).toStrictEqual(['/a', '/b']);
582582
const callback = vi.fn(() => {
@@ -622,7 +622,7 @@ describe('patchLayer', () => {
622622
storeLayer(req, '/a');
623623
storeLayer(req, '/b');
624624

625-
patchLayer(options, layer, '/c');
625+
patchLayer(() => options, layer, '/c');
626626
expect(() => {
627627
layer.handle(req, res);
628628
}).toThrowError('yur head asplode');

0 commit comments

Comments
 (0)