Skip to content

Commit 3b12a57

Browse files
committed
Fixup
1 parent 004c2b2 commit 3b12a57

4 files changed

Lines changed: 27 additions & 4 deletions

File tree

lib/internal/nsolid_diag.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -394,10 +394,10 @@ dc.subscribe('tracing:fastify.request.handler:start', ({ request, route }) => {
394394
// Subscribe to HTTP response finish to capture Express routes
395395
// Express sets req.route.path during request processing, available by response time
396396
dc.subscribe('http.server.response.finish', ({ request, response }) => {
397-
if (request?.route?.path && !request[nsolid_route_s]) {
397+
const routePath = request?.route?.path;
398+
if (typeof routePath === 'string' && !request[nsolid_route_s]) {
398399
// Compose full route path including mounted router prefixes
399400
const baseUrl = request.baseUrl || '';
400-
const routePath = request.route.path;
401401
// Avoid double slashes when concatenating (e.g., baseUrl ends with / and routePath starts with /)
402402
const fullPath = baseUrl.endsWith('/') && routePath.startsWith('/') ?
403403
baseUrl + routePath.slice(1) :

test/integrations/express/v4/test-http-route.mjs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Flags: --expose-internals
22
import { mustSucceed } from '../../../common/index.mjs';
3+
import { mustNotCall } from '../../../common/index.mjs';
34
import assert from 'node:assert';
45
import { existsSync } from 'node:fs';
56
import { fileURLToPath } from 'node:url';
@@ -110,6 +111,10 @@ async function runTest() {
110111
res.json({ status: 'ok' });
111112
});
112113

114+
app.get(/^\/regex\/(\d+)$/, (req, res) => {
115+
res.json({ userId: req.params[0] });
116+
});
117+
113118
// Track connections for forceful close
114119
const connections = new Set();
115120
const server = await new Promise((resolve) => {
@@ -148,6 +153,8 @@ async function runTest() {
148153
});
149154
});
150155

156+
process.once('uncaughtException', mustNotCall('regex Express routes must not throw in http.server.response.finish'));
157+
151158
// Make HTTP requests to trigger routes
152159
console.log('Making HTTP requests...');
153160

@@ -166,6 +173,11 @@ async function runTest() {
166173
assert.strictEqual(response.status, 200);
167174
console.log('Requested /health');
168175

176+
// Request to regex route - should not set http.route or throw
177+
response = await fetch(`http://127.0.0.1:${serverPort}/regex/789`);
178+
assert.strictEqual(response.status, 200);
179+
console.log('Requested /regex/789');
180+
169181
// Wait for all metrics to be reported
170182
await metricsPromise;
171183

test/integrations/express/v5/test-http-route.mjs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Flags: --expose-internals
22
import { mustSucceed } from '../../../common/index.mjs';
3+
import { mustNotCall } from '../../../common/index.mjs';
34
import assert from 'node:assert';
45
import { existsSync } from 'node:fs';
56
import { fileURLToPath } from 'node:url';
@@ -110,6 +111,10 @@ async function runTest() {
110111
res.json({ status: 'ok' });
111112
});
112113

114+
app.get(/^\/regex\/(\d+)$/, (req, res) => {
115+
res.json({ userId: req.params[0] });
116+
});
117+
113118
// Track connections for forceful close
114119
const connections = new Set();
115120
const server = await new Promise((resolve) => {
@@ -148,6 +153,8 @@ async function runTest() {
148153
});
149154
});
150155

156+
process.once('uncaughtException', mustNotCall('regex Express routes must not throw in http.server.response.finish'));
157+
151158
// Make HTTP requests to trigger routes
152159
console.log('Making HTTP requests...');
153160

@@ -166,6 +173,11 @@ async function runTest() {
166173
assert.strictEqual(response.status, 200);
167174
console.log('Requested /health');
168175

176+
// Request to regex route - should not set http.route or throw
177+
response = await fetch(`http://127.0.0.1:${serverPort}/regex/789`);
178+
assert.strictEqual(response.status, 200);
179+
console.log('Requested /regex/789');
180+
169181
// Wait for all metrics to be reported
170182
await metricsPromise;
171183

test/integrations/fastify/v5/test-http-route.mjs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ const __dirname = dirname(fileURLToPath(import.meta.url));
1818

1919
// Skip test if dependencies not installed
2020
if (!existsSync(join(__dirname, 'node_modules'))) {
21-
console.log('SKIP: node_modules not found. Run "make test-integrations-prereqs" first.');
22-
process.exit(0);
21+
throw new Error('SKIP: node_modules not found. Run "make test-integrations-prereqs" first.');
2322
}
2423

2524
const { default: Fastify } = await import('fastify');

0 commit comments

Comments
 (0)