Skip to content

Commit

Permalink
fix: Do not use wrap helper for req handler
Browse files Browse the repository at this point in the history
  • Loading branch information
timokoessler committed Dec 2, 2024
1 parent 31db21a commit 3641448
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 12 deletions.
29 changes: 29 additions & 0 deletions library/sources/Express.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,26 @@ export function createExpressTests(expressPackageName: string) {

app.use(newRouter);

app.use("/", express.static(__dirname + "/fixtures/public/"));

const nestedApp = express();
nestedApp.set("trust proxy", true);
nestedApp.get("/", (req, res) => {
res.send(getContext());
});

app.use("/nested-app", nestedApp);

const nestedNestedApp = express();
nestedNestedApp.get("/2", (req, res) => {
res.send(getContext());
});
nestedApp.use(nestedNestedApp);

nestedApp.get("/foo", (req, res) => {
res.send("bar");
});

app.get("/", (req, res) => {
const context = getContext();

Expand Down Expand Up @@ -608,6 +621,12 @@ export function createExpressTests(expressPackageName: string) {
});
});

t.test("it supports static files", async (t) => {
const response = await request(getApp()).get("/test.txt");

t.same(response.text, "Testfile");
});

t.test("it supports nested app", async (t) => {
const response = await request(getApp()).get("/nested-app");

Expand All @@ -616,6 +635,16 @@ export function createExpressTests(expressPackageName: string) {
source: "express",
route: "/nested-app",
});

const response2 = await request(getApp()).get("/nested-app/foo");
t.same(response2.text, "bar");

const response3 = await request(getApp()).get("/nested-app/2");
t.match(response3.body, {
method: "GET",
source: "express",
route: "/nested-app/:number",
});
});

// Express instrumentation results in routes with no stack, crashing Ghost
Expand Down
23 changes: 11 additions & 12 deletions library/sources/express/wrapRequestHandler.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
/* eslint-disable prefer-rest-params */
import type { RequestHandler } from "express";
import { runWithContext } from "../../agent/Context";
import { createWrappedFunction } from "../../helpers/wrap";
import { contextFromRequest } from "./contextFromRequest";

export function wrapRequestHandler(handler: RequestHandler): RequestHandler {
const fn = createWrappedFunction(handler, function wrap(handler) {
return function wrap(this: RequestHandler) {
if (arguments.length === 0) {
return handler.apply(this);
}
const fn = function wrap(this: RequestHandler) {
if (arguments.length === 0) {
// @ts-expect-error Type of this
return handler.apply(this);
}

Check warning on line 11 in library/sources/express/wrapRequestHandler.ts

View check run for this annotation

Codecov / codecov/patch

library/sources/express/wrapRequestHandler.ts#L9-L11

Added lines #L9 - L11 were not covered by tests

const context = contextFromRequest(arguments[0]);
const context = contextFromRequest(arguments[0]);

return runWithContext(context, () => {
return handler.apply(this, arguments);
});
};
}) as RequestHandler;
return runWithContext(context, () => {
// @ts-expect-error Type of arguments
return handler.apply(this, arguments);
});
};

// Some libraries/apps have properties on the handler functions that are not copied by our createWrappedFunction function
// (createWrappedFunction only copies properties when hasOwnProperty is true)
Expand Down
1 change: 1 addition & 0 deletions library/sources/fixtures/public/test.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Testfile

0 comments on commit 3641448

Please sign in to comment.