Skip to content

Commit b584929

Browse files
authored
Fix paths for complex Express apps (#50)
1 parent 00d4887 commit b584929

File tree

4 files changed

+46
-14
lines changed

4 files changed

+46
-14
lines changed

src/express/middleware.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ const getRoutePath = (req: Request) => {
138138
}
139139
if (req.baseUrl) {
140140
const routerPath = getRouterPath(req.app._router.stack, req.baseUrl);
141-
return routerPath + req.route.path;
141+
return req.route.path === "/" ? routerPath : routerPath + req.route.path;
142142
}
143143
return req.route.path;
144144
};
@@ -147,7 +147,8 @@ const getRouterPath = (stack: any[], baseUrl: string) => {
147147
const routerPaths: string[] = [];
148148
while (stack && stack.length > 0) {
149149
const routerLayer = stack.find(
150-
(layer) => layer.name === "router" && layer.regexp?.test(baseUrl),
150+
(layer) =>
151+
layer.name === "router" && layer.path && layer.regexp?.test(baseUrl),
151152
);
152153
if (routerLayer) {
153154
if (routerLayer.keys.length > 0) {
@@ -165,7 +166,7 @@ const getRouterPath = (stack: any[], baseUrl: string) => {
165166
break;
166167
}
167168
}
168-
return routerPaths.join("");
169+
return routerPaths.filter((path) => path !== "/").join("");
169170
};
170171

171172
const getConsumer = (req: Request) => {

src/express/utils.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ export const getEndpoints = function (app, basePath) {
234234
.filter((method) => !["HEAD", "OPTIONS"].includes(method.toUpperCase()))
235235
.map((method) => ({
236236
method,
237-
path: basePath + route.path,
237+
path: (basePath + route.path).replace(/\/\//g, "/"),
238238
})),
239239
);
240240
};

tests/express/app.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,19 @@ describe("Middleware for Express with nested routers", () => {
193193
});
194194

195195
it("Request logger", async () => {
196+
await appTest.get("/health").expect(200);
196197
await appTest.get("/api/v1/hello/bob").expect(200);
197198
await appTest.get("/api/v2/goodbye/world").expect(200);
199+
await appTest.get("/test").expect(200);
198200

199201
const requests = client.requestCounter.getAndResetRequests();
200-
expect(requests.length).toBe(2);
202+
expect(requests.length).toBe(4);
203+
expect(
204+
requests.some(
205+
(r) =>
206+
r.method === "GET" && r.path === "/health" && r.status_code === 200,
207+
),
208+
).toBe(true);
201209
expect(
202210
requests.some(
203211
(r) =>
@@ -214,10 +222,20 @@ describe("Middleware for Express with nested routers", () => {
214222
r.status_code === 200,
215223
),
216224
).toBe(true);
225+
expect(
226+
requests.some(
227+
(r) =>
228+
r.method === "GET" && r.path === "/test" && r.status_code === 200,
229+
),
230+
).toBe(true);
217231
});
218232

219233
it("List endpoints", async () => {
220234
expect(client.startupData?.paths).toEqual([
235+
{
236+
method: "GET",
237+
path: "/health",
238+
},
221239
{
222240
method: "GET",
223241
path: "/api/:version/hello/:name",
@@ -226,6 +244,10 @@ describe("Middleware for Express with nested routers", () => {
226244
method: "GET",
227245
path: "/api/:version/goodbye/world",
228246
},
247+
{
248+
method: "GET",
249+
path: "/test",
250+
},
229251
]);
230252
});
231253

tests/express/app.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ export const getAppWithValidator = () => {
104104
throw new Error("test");
105105
});
106106

107-
app.use(errors());
108107
return app;
109108
};
110109

@@ -124,31 +123,41 @@ export const getAppWithMiddlewareOnRouter = () => {
124123
});
125124

126125
app.use("/api", router);
127-
app.use(errors());
128126
return app;
129127
};
130128

131129
export const getAppWithNestedRouters = () => {
132130
const app = express();
133-
const router1 = express.Router({ mergeParams: true });
134-
const router2 = express.Router();
131+
const router1 = express.Router();
132+
const router2 = express.Router({ mergeParams: true });
133+
const router3 = express.Router();
134+
const router4 = express.Router();
135135

136136
useApitally(app, {
137137
clientId: CLIENT_ID,
138138
env: ENV,
139139
appVersion: "1.2.3",
140140
});
141141

142-
router1.get("/hello/:name", (req, res) => {
142+
router1.get("/health", (req, res) => {
143+
res.send("OK");
144+
});
145+
146+
router2.get("/hello/:name", (req, res) => {
143147
res.send(`Hello ${req.params.name}!`);
144148
});
145149

146-
router2.get("/world", (req, res) => {
150+
router3.get("/world", (req, res) => {
147151
res.send("World!");
148152
});
149153

150-
router1.use("/goodbye", router2);
151-
app.use("/api/:version", router1);
152-
app.use(errors());
154+
router4.get("/", (req, res) => {
155+
res.send("Success!");
156+
});
157+
158+
router2.use("/goodbye", router3);
159+
app.use("/", router1);
160+
app.use("/api/:version", router2);
161+
app.use("/test", router4);
153162
return app;
154163
};

0 commit comments

Comments
 (0)