Skip to content

Commit b0a600e

Browse files
committed
fix: record http client requests as self-contained call/return pairs
An http client request used to emit its call event when the request was sent and its return event when the response arrived. When two requests overlap, the second response can arrive before the first one's, so the events interleaved and a return event no longer closed the most recently opened call, breaking the nesting the AppMap format requires. Emit both events up front instead. Recording.httpClientRequest() now writes the call event and an incomplete return event, and hands the latter back to the caller. Once the response arrives the hook passes that event to Recording.httpClientResponse(), which fills in the status code, headers, body and elapsed time through the existing fixup mechanism. The pair is therefore always adjacent, whatever order the responses come in. Until (and unless) the response arrives the return event simply carries no http_client_response, so that field is now optional on HttpClientResponseEvent. A request that is aborted, errors out or outlives the recording ends up as a call/return pair with no response data, which is both accurate and still properly nested. Assisted-by: Claude:claude-opus-5
1 parent c125b65 commit b0a600e

5 files changed

Lines changed: 18 additions & 28 deletions

File tree

‎src/AppMap.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,8 @@ export interface HttpServerResponseEvent extends ReturnEventBase {
168168
}
169169

170170
export interface HttpClientResponseEvent extends ReturnEventBase {
171-
// status_code is missing if the response hasn't arrived (yet)
172-
http_client_response: Omit<HttpResponse, "status_code"> & { status_code?: number };
171+
// absent until the response arrives — and permanently, if it never does
172+
http_client_response?: HttpResponse;
173173
}
174174

175175
export interface SqlQueryEvent extends CallEventBase {

‎src/Recording.ts‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,15 @@ export default class Recording {
9999
return event;
100100
}
101101

102+
// Both the call and the return event are emitted right away, so that the events
103+
// nest properly even if requests overlap. The returned event carries no response
104+
// data yet; pass it to httpClientResponse() once the response arrives.
102105
httpClientRequest(
103106
method: string,
104107
url: string,
105108
headers?: Record<string, string>,
106-
): AppMap.HttpClientRequestEvent {
107-
const event: AppMap.HttpClientRequestEvent = {
109+
): AppMap.HttpClientResponseEvent {
110+
const call: AppMap.HttpClientRequestEvent = {
108111
event: "call",
109112
http_client_request: compactObject({
110113
request_method: method,
@@ -114,28 +117,20 @@ export default class Recording {
114117
id: this.nextId++,
115118
thread_id: 0,
116119
};
117-
this.emit(event);
118-
119-
return event;
120-
}
120+
this.emit(call);
121121

122-
// Emits the return event for an http client request immediately after the call,
123-
// so that overlapping requests don't break event nesting. The response data
124-
// is filled in later with httpClientResponseFixup once it arrives.
125-
httpClientResponse(callId: number): AppMap.HttpClientResponseEvent {
126-
const event: AppMap.HttpClientResponseEvent = {
122+
const response: AppMap.HttpClientResponseEvent = {
127123
event: "return",
128-
http_client_response: {},
129124
id: this.nextId++,
130125
thread_id: 0,
131-
parent_id: callId,
126+
parent_id: call.id,
132127
};
133-
this.emit(event);
128+
this.emit(response);
134129

135-
return event;
130+
return response;
136131
}
137132

138-
httpClientResponseFixup(
133+
httpClientResponse(
139134
returnEvent: AppMap.HttpClientResponseEvent,
140135
elapsed: number,
141136
status: number,

‎src/hooks/http.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,9 @@ function handleClientRequest(request: http.ClientRequest) {
112112

113113
const urlString = `${url.protocol}//${url.host}${url.pathname}`;
114114
const headers = normalizeHeaders(request.getHeaders());
115-
// Emit both the call and the return right away so that the events nest properly
116-
// even if multiple requests overlap; the return is fixed up once the response arrives.
117-
const clientResponseEvents = recordings.map((r) => {
118-
const call = r.httpClientRequest(request.method, urlString, headers);
119-
return r.httpClientResponse(call.id);
120-
});
115+
const clientResponseEvents = recordings.map((r) =>
116+
r.httpClientRequest(request.method, urlString, headers),
117+
);
121118

122119
request.on("response", (response) => {
123120
const capture = new BodyCapture();
@@ -141,7 +138,7 @@ function handleClientRequest(request: http.ClientRequest) {
141138
assert(response.statusCode != undefined);
142139
if (!isActive(recording)) return;
143140

144-
recording.httpClientResponseFixup(
141+
recording.httpClientResponse(
145142
clientResponseEvents[idx],
146143
elapsed,
147144
response.statusCode,

‎test/__snapshots__/httpClient.test.ts.snap‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,7 +1075,6 @@ exports[`mapping overlapping http client requests 1`] = `
10751075
},
10761076
{
10771077
"event": "return",
1078-
"http_client_response": {},
10791078
"id": 6,
10801079
"parent_id": 5,
10811080
"thread_id": 0,
@@ -1094,7 +1093,6 @@ exports[`mapping overlapping http client requests 1`] = `
10941093
},
10951094
{
10961095
"event": "return",
1097-
"http_client_response": {},
10981096
"id": 8,
10991097
"parent_id": 7,
11001098
"thread_id": 0,

‎test/nextApp.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export default async function testNextApp() {
4040
(a) =>
4141
a.events?.forEach((e) => {
4242
if ("http_server_response" in e) delete e.http_server_response.return_value;
43-
if ("http_client_response" in e) delete e.http_client_response.return_value;
43+
if ("http_client_response" in e) delete e.http_client_response?.return_value;
4444
}),
4545
);
4646

0 commit comments

Comments
 (0)