Skip to content

Commit 45aed9a

Browse files
mydeamydea
committed
fix and improve tests
1 parent 2fb64b7 commit 45aed9a

2 files changed

Lines changed: 106 additions & 160 deletions

File tree

‎dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts‎

Lines changed: 34 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
getSpanOp,
66
collectStreamedSpansUntilSegment,
77
} from '@sentry-internal/test-utils';
8-
import { APP_NAME, RUNTIME } from './constants';
8+
import { APP_NAME } from './constants';
99

1010
test.describe('route handler errors', () => {
1111
test('captures error with mechanism and trace correlation', async ({ baseURL }) => {
@@ -103,81 +103,37 @@ test.describe('HTTPException errors', () => {
103103
});
104104
});
105105

106-
// On Node/Bun, httpServerSpansIntegration drops transactions for 3xx/4xx responses (ignoreStatusCodes), so we just use a request guard.
107-
// On Cloudflare the transaction is available, and we additionally verify its name.
108-
[301, 302].forEach(code => {
109-
test(`does not capture ${code} HTTPException`, async ({ baseURL }) => {
110-
let errorEventOccurred = false;
111-
112-
waitForError(APP_NAME, event => {
113-
if (event.exception?.values?.[0]?.value === `HTTPException ${code}`) {
114-
errorEventOccurred = true;
115-
}
116-
return false;
117-
});
118-
119-
const segmentPromise = waitForStreamedSpan(
120-
APP_NAME,
121-
segment =>
122-
segment.is_segment &&
123-
(RUNTIME === 'cloudflare'
124-
? getSpanOp(segment) === 'http.server' && !!segment.name?.includes('/http-exception/')
125-
: getSpanOp(segment) === 'http.server' && segment.name === 'GET /'),
126-
);
127-
128-
const response = await fetch(`${baseURL}/http-exception/${code}`, { redirect: 'manual' });
129-
expect(response.status).toBe(code);
130-
131-
if (RUNTIME !== 'cloudflare') {
132-
// Simple request guard for non-Cloudflare runtimes since the other transaction is dropped for 4xx responses
133-
await fetch(`${baseURL}/`);
134-
}
135-
136-
const segment = await segmentPromise;
137-
138-
if (RUNTIME === 'cloudflare') {
139-
expect(segment.name).toBe('GET /http-exception/:code');
106+
// 3xx/4xx responses must not be captured as errors. Some runtimes drop the transaction for those
107+
// status codes (httpServerSpansIntegration's ignoreStatusCodes), so instead of waiting on the
108+
// HTTPException route's own (possibly dropped) transaction, we wait on a defined 2xx route's
109+
// transaction as a flush guard — that one is produced on every runtime — then assert no error was
110+
// captured. (Parametrized route naming is covered by tracing.test.ts on 2xx routes.)
111+
const expectHttpExceptionNotCaptured = async (baseURL: string, code: number): Promise<void> => {
112+
let errorEventOccurred = false;
113+
waitForError(APP_NAME, event => {
114+
if (event.exception?.values?.[0]?.value === `HTTPException ${code}`) {
115+
errorEventOccurred = true;
140116
}
141-
142-
expect(errorEventOccurred).toBe(false);
117+
return false;
143118
});
144-
});
145119

146-
[401, 403, 404].forEach(code => {
147-
test(`does not capture ${code} HTTPException`, async ({ baseURL }) => {
148-
let errorEventOccurred = false;
149-
150-
waitForError(APP_NAME, event => {
151-
if (event.exception?.values?.[0]?.value === `HTTPException ${code}`) {
152-
errorEventOccurred = true;
153-
}
154-
return false;
155-
});
156-
157-
const segmentPromise = waitForStreamedSpan(
158-
APP_NAME,
159-
segment =>
160-
segment.is_segment &&
161-
(RUNTIME === 'cloudflare'
162-
? getSpanOp(segment) === 'http.server' && !!segment.name?.includes('/http-exception/')
163-
: getSpanOp(segment) === 'http.server' && segment.name === 'GET /'),
164-
);
165-
166-
const response = await fetch(`${baseURL}/http-exception/${code}`);
167-
expect(response.status).toBe(code);
168-
169-
if (RUNTIME !== 'cloudflare') {
170-
// Simple request guard for non-Cloudflare runtimes since the other transaction is dropped for 4xx responses
171-
await fetch(`${baseURL}/`);
172-
}
120+
const guardPromise = waitForStreamedSpan(
121+
APP_NAME,
122+
segment => segment.is_segment && getSpanOp(segment) === 'http.server' && segment.name === 'GET /',
123+
);
173124

174-
const segment = await segmentPromise;
125+
const response = await fetch(`${baseURL}/http-exception/${code}`, { redirect: 'manual' });
126+
expect(response.status).toBe(code);
175127

176-
if (RUNTIME === 'cloudflare') {
177-
expect(segment.name).toBe('GET /http-exception/:code');
178-
}
128+
await fetch(`${baseURL}/`);
129+
await guardPromise;
130+
131+
expect(errorEventOccurred).toBe(false);
132+
};
179133

180-
expect(errorEventOccurred).toBe(false);
134+
[301, 302, 401, 403, 404].forEach(code => {
135+
test(`does not capture ${code} HTTPException`, async ({ baseURL }) => {
136+
await expectHttpExceptionNotCaptured(baseURL!, code);
181137
});
182138
});
183139
});
@@ -226,41 +182,18 @@ test.describe('middleware errors', () => {
226182
return false;
227183
});
228184

229-
const segmentPromise = collectStreamedSpansUntilSegment(APP_NAME, segment => {
230-
if (RUNTIME === 'cloudflare') {
231-
return (
232-
getSpanOp(segment) === 'http.server' && !!segment.name?.includes('/test-errors/middleware-http-exception-4xx')
233-
);
234-
}
235-
return getSpanOp(segment) === 'http.server' && segment.name === 'GET /';
236-
});
185+
// Guard on a defined 2xx route's transaction (produced on every runtime) rather than the 4xx
186+
// route's own transaction, which some runtimes drop — then assert no error was captured.
187+
const guardPromise = waitForStreamedSpan(
188+
APP_NAME,
189+
segment => segment.is_segment && getSpanOp(segment) === 'http.server' && segment.name === 'GET /',
190+
);
237191

238192
const response = await fetch(`${baseURL}/test-errors/middleware-http-exception-4xx`);
239193
expect(response.status).toBe(401);
240194

241-
if (RUNTIME !== 'cloudflare') {
242-
await fetch(`${baseURL}/`);
243-
}
244-
245-
const segmentSpans = await segmentPromise;
246-
const segment = segmentSpans.find(segment => {
247-
if (!segment.is_segment) return false;
248-
if (RUNTIME === 'cloudflare') {
249-
return (
250-
getSpanOp(segment) === 'http.server' && !!segment.name?.includes('/test-errors/middleware-http-exception-4xx')
251-
);
252-
}
253-
return getSpanOp(segment) === 'http.server' && segment.name === 'GET /';
254-
})!;
255-
256-
if (RUNTIME === 'cloudflare') {
257-
expect(segment.name).toBe('GET /test-errors/middleware-http-exception-4xx');
258-
259-
const middlewareSpan = segmentSpans
260-
.filter(span => !span.is_segment && span.attributes['sentry.segment.id']?.value === segment.span_id)
261-
.find(s => getSpanOp(s) === 'middleware');
262-
expect(middlewareSpan?.status).not.toBe('error');
263-
}
195+
await fetch(`${baseURL}/`);
196+
await guardPromise;
264197

265198
expect(errorEventOccurred).toBe(false);
266199
});

‎dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts‎

Lines changed: 72 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,65 @@
11
import { expect, test } from '@playwright/test';
22
import { waitForStreamedSpan, getSpanOp } from '@sentry-internal/test-utils';
3-
import { APP_NAME, RUNTIME } from './constants';
3+
import { APP_NAME, RUNTIME, type Runtime } from './constants';
4+
5+
const anyString = expect.any(String) as unknown;
6+
const anyNumber = expect.any(Number) as unknown;
7+
const ipvType = expect.stringMatching(/^ipv[46]$/) as unknown;
8+
9+
// Per-runtime `http.server` connection-info expectations. Each runtime's `getConnInfo` helper exposes
10+
// a different set of fields, so the expected attribute values are declared here once (keyed by
11+
// runtime) instead of branching inside the tests. `undefined` means the attribute must be absent.
12+
// Relational checks (`network.peer.*` mirroring `client.*`) are asserted in the tests, since they
13+
// hold on every runtime (including when both sides are absent).
14+
//
15+
// - `added`: attributes Hono's conninfo middleware contributes.
16+
// - `baseline`: attributes the SDK sends independent of conninfo (regression guard that conninfo only
17+
// adds, never clobbers).
18+
const CONN_INFO: Record<Runtime, { added: Record<string, unknown>; baseline: Record<string, unknown> }> = {
19+
node: {
20+
added: { 'client.port': anyNumber, 'network.type': ipvType, 'network.transport': undefined },
21+
baseline: {
22+
'server.address': 'localhost',
23+
'server.port': anyNumber,
24+
'client.address': anyString,
25+
'client.port': anyNumber,
26+
'network.type': ipvType,
27+
'network.protocol.name': 'http',
28+
'network.protocol.version': '1.1',
29+
'network.transport': undefined,
30+
'network.local.address': anyString,
31+
'network.local.port': anyNumber,
32+
},
33+
},
34+
bun: {
35+
added: { 'client.port': anyNumber, 'network.type': ipvType, 'network.transport': undefined },
36+
baseline: { 'client.address': anyString, 'client.port': anyNumber, 'network.type': ipvType },
37+
},
38+
deno: {
39+
// Only `hono/deno` exposes `network.transport`.
40+
added: { 'client.port': anyNumber, 'network.transport': expect.stringMatching(/tcp/) },
41+
baseline: {
42+
'server.address': 'localhost',
43+
'client.address': anyString,
44+
'client.port': anyNumber,
45+
'network.transport': 'tcp',
46+
'network.protocol.name': 'http',
47+
},
48+
},
49+
cloudflare: {
50+
// Cloudflare Workers expose no port, address family, or transport. Asserting their absence lets
51+
// us notice if that ever changes.
52+
added: { 'client.port': undefined, 'network.type': undefined, 'network.transport': undefined },
53+
baseline: {
54+
'server.address': 'localhost',
55+
'client.address': '::1',
56+
'network.protocol.name': 'http',
57+
'network.protocol.version': '1.1',
58+
},
59+
},
60+
};
61+
62+
const connInfo = CONN_INFO[RUNTIME];
463

564
test('sends a span for the index route', async ({ baseURL }) => {
665
const segmentPromise = waitForStreamedSpan(
@@ -46,31 +105,14 @@ test('attaches HTTP connection info to the server span', async ({ baseURL, page
46105
const data = segment.attributes ?? {};
47106

48107
expect(data['client.address']?.value).toEqual(expect.any(String));
49-
expect(data['network.peer.address']?.value).toBe(data['client.address']?.value);
50108

51-
if (RUNTIME !== 'deno') {
52-
// Only exposed in `hono/deno`
53-
expect(data['network.transport']?.value).toBeUndefined();
54-
} else {
55-
expect(data['network.transport']?.value).toMatch(/tcp/);
56-
}
109+
// conninfo must only *add* attributes, never replace: peer mirrors client on every runtime
110+
// (including when both are absent).
111+
expect(data['network.peer.address']?.value).toBe(data['client.address']?.value);
112+
expect(data['network.peer.port']?.value).toBe(data['client.port']?.value);
57113

58-
if (RUNTIME === 'node' || RUNTIME === 'bun') {
59-
// Node (@hono/node-server) and Bun expose socket-level port and address family.
60-
expect(data['client.port']?.value).toEqual(expect.any(Number));
61-
expect(data['network.peer.port']?.value).toBe(data['client.port']?.value);
62-
expect(data['network.type']?.value).toMatch(/^ipv[46]$/);
63-
} else if (RUNTIME === 'deno') {
64-
expect(data['client.port']?.value).toEqual(expect.any(Number));
65-
expect(data['network.peer.port']?.value).toBe(data['client.port']?.value);
66-
} else if (RUNTIME === 'cloudflare') {
67-
// Cloudflare Workers expose no port, address family, or transport.
68-
// This could change in the future and checking for the absence of these fields allows us to notice if/when that happens.
69-
expect(data['client.port']?.value).toBeUndefined();
70-
expect(data['network.peer.port']?.value).toBeUndefined();
71-
expect(data['network.type']?.value).toBeUndefined();
72-
} else {
73-
throw new Error(`No tests for runtime: ${RUNTIME}`);
114+
for (const [key, expected] of Object.entries(connInfo.added)) {
115+
expect(data[key]?.value).toEqual(expected);
74116
}
75117
});
76118

@@ -91,42 +133,13 @@ test("preserves the baseline server.*, client.* and network.* server span attrib
91133
const segment = await segmentPromise;
92134
const data = segment.attributes ?? {};
93135

94-
if (RUNTIME === 'node') {
95-
expect(data['server.address']?.value).toBe('localhost');
96-
expect(data['server.port']?.value).toBe(Number(new URL(baseURL!).port));
97-
expect(data['client.address']?.value).toEqual(expect.any(String));
98-
expect(data['client.port']?.value).toEqual(expect.any(Number));
99-
expect(data['network.type']?.value).toMatch(/^ipv[46]$/);
100-
expect(data['network.protocol.name']?.value).toBe('http');
101-
expect(data['network.protocol.version']?.value).toBe('1.1');
102-
expect(data['network.transport']?.value).toBeUndefined();
103-
expect(data['network.local.port']?.value).toBe(data['server.port']?.value);
104-
expect(data['network.local.address']?.value).toEqual(expect.any(String));
105-
expect(data['network.peer.address']?.value).toBe(data['client.address']?.value);
106-
expect(data['network.peer.port']?.value).toBe(data['client.port']?.value);
107-
} else if (RUNTIME === 'bun') {
108-
expect(data['client.address']?.value).toEqual(expect.any(String));
109-
expect(data['client.port']?.value).toEqual(expect.any(Number));
110-
expect(data['network.peer.address']?.value).toBe(data['client.address']?.value);
111-
expect(data['network.peer.port']?.value).toBe(data['client.port']?.value);
112-
expect(data['network.type']?.value).toMatch(/^ipv[46]$/);
113-
} else if (RUNTIME === 'cloudflare') {
114-
expect(data['server.address']?.value).toBe('localhost');
115-
expect(data['client.address']?.value).toBe('::1');
116-
expect(data['network.peer.address']?.value).toBe(data['client.address']?.value);
117-
expect(data['network.protocol.name']?.value).toBe('http');
118-
expect(data['network.protocol.version']?.value).toBe('1.1');
119-
} else if (RUNTIME === 'deno') {
120-
expect(data['server.address']?.value).toBe('localhost');
121-
expect(data['client.address']?.value).toEqual(expect.any(String));
122-
expect(data['client.port']?.value).toEqual(expect.any(Number));
123-
expect(data['network.peer.address']?.value).toBe(data['client.address']?.value);
124-
expect(data['network.peer.port']?.value).toBe(data['client.port']?.value);
125-
expect(data['network.transport']?.value).toBe('tcp');
126-
expect(data['network.protocol.name']?.value).toBe('http');
127-
} else {
128-
throw new Error(`No tests for runtime: ${RUNTIME}`);
136+
for (const [key, expected] of Object.entries(connInfo.baseline)) {
137+
expect(data[key]?.value).toEqual(expected);
129138
}
139+
140+
// Relational checks that hold on every runtime (both sides absent → still equal).
141+
expect(data['network.peer.address']?.value).toBe(data['client.address']?.value);
142+
expect(data['network.peer.port']?.value).toBe(data['client.port']?.value);
130143
});
131144

132145
test('sends a span for a route that throws', async ({ baseURL }) => {

0 commit comments

Comments
 (0)