Skip to content

Commit e80da53

Browse files
timfishtimfish
authored
ref(node): Compression support for node http transport (#5139)
1 parent 8dbe905 commit e80da53

File tree

3 files changed

+168
-99
lines changed

3 files changed

+168
-99
lines changed

‎packages/node/src/transports/http-module.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { IncomingHttpHeaders, RequestOptions as HTTPRequestOptions } from 'http';
22
import { RequestOptions as HTTPSRequestOptions } from 'https';
3+
import { Writable } from 'stream';
34
import { URL } from 'url';
45

56
export type HTTPModuleRequestOptions = HTTPRequestOptions | HTTPSRequestOptions | string | URL;
@@ -15,15 +16,6 @@ export interface HTTPModuleRequestIncomingMessage {
1516
setEncoding(encoding: string): void;
1617
}
1718

18-
/**
19-
* Cut version of http.ClientRequest.
20-
* Some transports work in a special Javascript environment where http.IncomingMessage is not available.
21-
*/
22-
export interface HTTPModuleClientRequest {
23-
end(chunk: string | Uint8Array): void;
24-
on(event: 'error', listener: () => void): void;
25-
}
26-
2719
/**
2820
* Internal used interface for typescript.
2921
* @hidden
@@ -34,10 +26,7 @@ export interface HTTPModule {
3426
* @param options These are {@see TransportOptions}
3527
* @param callback Callback when request is finished
3628
*/
37-
request(
38-
options: HTTPModuleRequestOptions,
39-
callback?: (res: HTTPModuleRequestIncomingMessage) => void,
40-
): HTTPModuleClientRequest;
29+
request(options: HTTPModuleRequestOptions, callback?: (res: HTTPModuleRequestIncomingMessage) => void): Writable;
4130

4231
// This is the type for nodejs versions that handle the URL argument
4332
// (v10.9.0+), but we do not use it just yet because we support older node

‎packages/node/src/transports/http.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import {
88
} from '@sentry/types';
99
import * as http from 'http';
1010
import * as https from 'https';
11+
import { Readable } from 'stream';
1112
import { URL } from 'url';
13+
import { createGzip } from 'zlib';
1214

1315
import { HTTPModule } from './http-module';
1416

@@ -23,6 +25,22 @@ export interface NodeTransportOptions extends BaseTransportOptions {
2325
httpModule?: HTTPModule;
2426
}
2527

28+
// Estimated maximum size for reasonable standalone event
29+
const GZIP_THRESHOLD = 1024 * 32;
30+
31+
/**
32+
* Gets a stream from a Uint8Array or string
33+
* Readable.from is ideal but was added in node.js v12.3.0 and v10.17.0
34+
*/
35+
function streamFromBody(body: Uint8Array | string): Readable {
36+
return new Readable({
37+
read() {
38+
this.push(body);
39+
this.push(null);
40+
},
41+
});
42+
}
43+
2644
/**
2745
* Creates a Transport that uses native the native 'http' and 'https' modules to send events to Sentry.
2846
*/
@@ -85,6 +103,17 @@ function createRequestExecutor(
85103
const { hostname, pathname, port, protocol, search } = new URL(options.url);
86104
return function makeRequest(request: TransportRequest): Promise<TransportMakeRequestResponse> {
87105
return new Promise((resolve, reject) => {
106+
let body = streamFromBody(request.body);
107+
108+
if (request.body.length > GZIP_THRESHOLD) {
109+
options.headers = {
110+
...options.headers,
111+
'content-encoding': 'gzip',
112+
};
113+
114+
body = body.pipe(createGzip());
115+
}
116+
88117
const req = httpModule.request(
89118
{
90119
method: 'POST',
@@ -123,7 +152,7 @@ function createRequestExecutor(
123152
);
124153

125154
req.on('error', reject);
126-
req.end(request.body);
155+
body.pipe(req);
127156
});
128157
};
129158
}

‎packages/node/test/transports/http.test.ts

Lines changed: 136 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { createTransport } from '@sentry/core';
22
import { EventEnvelope, EventItem } from '@sentry/types';
3-
import { createEnvelope, serializeEnvelope } from '@sentry/utils';
3+
import { addItemToEnvelope, createAttachmentEnvelopeItem, createEnvelope, serializeEnvelope } from '@sentry/utils';
44
import * as http from 'http';
55
import { TextEncoder } from 'util';
6+
import { createGunzip } from 'zlib';
67

78
import { makeNodeTransport } from '../../src/transports';
89

@@ -34,17 +35,21 @@ let testServer: http.Server | undefined;
3435

3536
function setupTestServer(
3637
options: TestServerOptions,
37-
requestInspector?: (req: http.IncomingMessage, body: string) => void,
38+
requestInspector?: (req: http.IncomingMessage, body: string, raw: Uint8Array) => void,
3839
) {
3940
testServer = http.createServer((req, res) => {
40-
let body = '';
41+
const chunks: Buffer[] = [];
4142

42-
req.on('data', data => {
43-
body += data;
43+
const stream = req.headers['content-encoding'] === 'gzip' ? req.pipe(createGunzip({})) : req;
44+
45+
stream.on('error', () => {});
46+
47+
stream.on('data', data => {
48+
chunks.push(data);
4449
});
4550

46-
req.on('end', () => {
47-
requestInspector?.(req, body);
51+
stream.on('end', () => {
52+
requestInspector?.(req, chunks.join(), Buffer.concat(chunks));
4853
});
4954

5055
res.writeHead(options.statusCode, options.responseHeaders);
@@ -69,6 +74,16 @@ const EVENT_ENVELOPE = createEnvelope<EventEnvelope>({ event_id: 'aa3ff046696b4b
6974

7075
const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, new TextEncoder());
7176

77+
const ATTACHMENT_ITEM = createAttachmentEnvelopeItem(
78+
{ filename: 'empty-file.bin', data: new Uint8Array(50_000) },
79+
new TextEncoder(),
80+
);
81+
const EVENT_ATTACHMENT_ENVELOPE = addItemToEnvelope(EVENT_ENVELOPE, ATTACHMENT_ITEM);
82+
const SERIALIZED_EVENT_ATTACHMENT_ENVELOPE = serializeEnvelope(
83+
EVENT_ATTACHMENT_ENVELOPE,
84+
new TextEncoder(),
85+
) as Uint8Array;
86+
7287
const defaultOptions = {
7388
url: TEST_SERVER_URL,
7489
recordDroppedEvent: () => undefined,
@@ -155,6 +170,40 @@ describe('makeNewHttpTransport()', () => {
155170
});
156171
});
157172

173+
describe('compression', () => {
174+
it('small envelopes should not be compressed', async () => {
175+
await setupTestServer(
176+
{
177+
statusCode: SUCCESS,
178+
responseHeaders: {},
179+
},
180+
(req, body) => {
181+
expect(req.headers['content-encoding']).toBeUndefined();
182+
expect(body).toBe(SERIALIZED_EVENT_ENVELOPE);
183+
},
184+
);
185+
186+
const transport = makeNodeTransport(defaultOptions);
187+
await transport.send(EVENT_ENVELOPE);
188+
});
189+
190+
it('large envelopes should be compressed', async () => {
191+
await setupTestServer(
192+
{
193+
statusCode: SUCCESS,
194+
responseHeaders: {},
195+
},
196+
(req, _, raw) => {
197+
expect(req.headers['content-encoding']).toEqual('gzip');
198+
expect(raw.buffer).toStrictEqual(SERIALIZED_EVENT_ATTACHMENT_ENVELOPE.buffer);
199+
},
200+
);
201+
202+
const transport = makeNodeTransport(defaultOptions);
203+
await transport.send(EVENT_ATTACHMENT_ENVELOPE);
204+
});
205+
});
206+
158207
describe('proxy', () => {
159208
it('can be configured through option', () => {
160209
makeNodeTransport({
@@ -236,104 +285,106 @@ describe('makeNewHttpTransport()', () => {
236285
});
237286
});
238287

239-
it('should register TransportRequestExecutor that returns the correct object from server response (rate limit)', async () => {
240-
await setupTestServer({
241-
statusCode: RATE_LIMIT,
242-
responseHeaders: {},
243-
});
244-
245-
makeNodeTransport(defaultOptions);
246-
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
247-
248-
const executorResult = registeredRequestExecutor({
249-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
250-
category: 'error',
251-
});
252-
253-
await expect(executorResult).resolves.toEqual(
254-
expect.objectContaining({
288+
describe('should register TransportRequestExecutor that returns the correct object from server response', () => {
289+
it('rate limit', async () => {
290+
await setupTestServer({
255291
statusCode: RATE_LIMIT,
256-
}),
257-
);
258-
});
292+
responseHeaders: {},
293+
});
259294

260-
it('should register TransportRequestExecutor that returns the correct object from server response (OK)', async () => {
261-
await setupTestServer({
262-
statusCode: SUCCESS,
263-
});
295+
makeNodeTransport(defaultOptions);
296+
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
264297

265-
makeNodeTransport(defaultOptions);
266-
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
298+
const executorResult = registeredRequestExecutor({
299+
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
300+
category: 'error',
301+
});
267302

268-
const executorResult = registeredRequestExecutor({
269-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
270-
category: 'error',
303+
await expect(executorResult).resolves.toEqual(
304+
expect.objectContaining({
305+
statusCode: RATE_LIMIT,
306+
}),
307+
);
271308
});
272309

273-
await expect(executorResult).resolves.toEqual(
274-
expect.objectContaining({
310+
it('OK', async () => {
311+
await setupTestServer({
275312
statusCode: SUCCESS,
276-
headers: {
277-
'retry-after': null,
278-
'x-sentry-rate-limits': null,
279-
},
280-
}),
281-
);
282-
});
313+
});
283314

284-
it('should register TransportRequestExecutor that returns the correct object from server response (OK with rate-limit headers)', async () => {
285-
await setupTestServer({
286-
statusCode: SUCCESS,
287-
responseHeaders: {
288-
'Retry-After': '2700',
289-
'X-Sentry-Rate-Limits': '60::organization, 2700::organization',
290-
},
291-
});
315+
makeNodeTransport(defaultOptions);
316+
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
292317

293-
makeNodeTransport(defaultOptions);
294-
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
318+
const executorResult = registeredRequestExecutor({
319+
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
320+
category: 'error',
321+
});
295322

296-
const executorResult = registeredRequestExecutor({
297-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
298-
category: 'error',
323+
await expect(executorResult).resolves.toEqual(
324+
expect.objectContaining({
325+
statusCode: SUCCESS,
326+
headers: {
327+
'retry-after': null,
328+
'x-sentry-rate-limits': null,
329+
},
330+
}),
331+
);
299332
});
300333

301-
await expect(executorResult).resolves.toEqual(
302-
expect.objectContaining({
334+
it('OK with rate-limit headers', async () => {
335+
await setupTestServer({
303336
statusCode: SUCCESS,
304-
headers: {
305-
'retry-after': '2700',
306-
'x-sentry-rate-limits': '60::organization, 2700::organization',
337+
responseHeaders: {
338+
'Retry-After': '2700',
339+
'X-Sentry-Rate-Limits': '60::organization, 2700::organization',
307340
},
308-
}),
309-
);
310-
});
341+
});
311342

312-
it('should register TransportRequestExecutor that returns the correct object from server response (NOK with rate-limit headers)', async () => {
313-
await setupTestServer({
314-
statusCode: RATE_LIMIT,
315-
responseHeaders: {
316-
'Retry-After': '2700',
317-
'X-Sentry-Rate-Limits': '60::organization, 2700::organization',
318-
},
319-
});
343+
makeNodeTransport(defaultOptions);
344+
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
320345

321-
makeNodeTransport(defaultOptions);
322-
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
346+
const executorResult = registeredRequestExecutor({
347+
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
348+
category: 'error',
349+
});
323350

324-
const executorResult = registeredRequestExecutor({
325-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
326-
category: 'error',
351+
await expect(executorResult).resolves.toEqual(
352+
expect.objectContaining({
353+
statusCode: SUCCESS,
354+
headers: {
355+
'retry-after': '2700',
356+
'x-sentry-rate-limits': '60::organization, 2700::organization',
357+
},
358+
}),
359+
);
327360
});
328361

329-
await expect(executorResult).resolves.toEqual(
330-
expect.objectContaining({
362+
it('NOK with rate-limit headers', async () => {
363+
await setupTestServer({
331364
statusCode: RATE_LIMIT,
332-
headers: {
333-
'retry-after': '2700',
334-
'x-sentry-rate-limits': '60::organization, 2700::organization',
365+
responseHeaders: {
366+
'Retry-After': '2700',
367+
'X-Sentry-Rate-Limits': '60::organization, 2700::organization',
335368
},
336-
}),
337-
);
369+
});
370+
371+
makeNodeTransport(defaultOptions);
372+
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
373+
374+
const executorResult = registeredRequestExecutor({
375+
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
376+
category: 'error',
377+
});
378+
379+
await expect(executorResult).resolves.toEqual(
380+
expect.objectContaining({
381+
statusCode: RATE_LIMIT,
382+
headers: {
383+
'retry-after': '2700',
384+
'x-sentry-rate-limits': '60::organization, 2700::organization',
385+
},
386+
}),
387+
);
388+
});
338389
});
339390
});

0 commit comments

Comments
 (0)