Skip to content

feat(node): Compression support for http transport #5209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions packages/node/src/transports/http-module.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { IncomingHttpHeaders, RequestOptions as HTTPRequestOptions } from 'http';
import { RequestOptions as HTTPSRequestOptions } from 'https';
import { Writable } from 'stream';
import { URL } from 'url';

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

/**
* Cut version of http.ClientRequest.
* Some transports work in a special Javascript environment where http.IncomingMessage is not available.
*/
export interface HTTPModuleClientRequest {
end(chunk: string | Uint8Array): void;
on(event: 'error', listener: () => void): void;
}

/**
* Internal used interface for typescript.
* @hidden
Expand All @@ -34,10 +26,7 @@ export interface HTTPModule {
* @param options These are {@see TransportOptions}
* @param callback Callback when request is finished
*/
request(
options: HTTPModuleRequestOptions,
callback?: (res: HTTPModuleRequestIncomingMessage) => void,
): HTTPModuleClientRequest;
request(options: HTTPModuleRequestOptions, callback?: (res: HTTPModuleRequestIncomingMessage) => void): Writable;

// This is the type for nodejs versions that handle the URL argument
// (v10.9.0+), but we do not use it just yet because we support older node
Expand Down
31 changes: 29 additions & 2 deletions packages/node/src/transports/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import {
} from '@sentry/types';
import * as http from 'http';
import * as https from 'https';
import { Readable } from 'stream';
import { URL } from 'url';
import { createGzip } from 'zlib';

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

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

// Estimated maximum size for reasonable standalone event
const GZIP_THRESHOLD = 1024 * 32;

/**
* Gets a stream from a Uint8Array or string
* Readable.from is ideal but was added in node.js v12.3.0 and v10.17.0
*/
function streamFromBody(body: Uint8Array | string): Readable {
return new Readable({
read() {
this.push(body);
this.push(null);
},
});
}

/**
* Creates a Transport that uses native the native 'http' and 'https' modules to send events to Sentry.
*/
Expand Down Expand Up @@ -85,11 +103,20 @@ function createRequestExecutor(
const { hostname, pathname, port, protocol, search } = new URL(options.url);
return function makeRequest(request: TransportRequest): Promise<TransportMakeRequestResponse> {
return new Promise((resolve, reject) => {
let body = streamFromBody(request.body);

const headers: Record<string, string> = { ...options.headers };

if (request.body.length > GZIP_THRESHOLD) {
headers['content-encoding'] = 'gzip';
body = body.pipe(createGzip());
}

const req = httpModule.request(
{
method: 'POST',
agent,
headers: options.headers,
headers,
hostname,
path: `${pathname}${search}`,
port,
Expand Down Expand Up @@ -123,7 +150,7 @@ function createRequestExecutor(
);

req.on('error', reject);
req.end(request.body);
body.pipe(req);
});
};
}
219 changes: 134 additions & 85 deletions packages/node/test/transports/http.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { createTransport } from '@sentry/core';
import { EventEnvelope, EventItem } from '@sentry/types';
import { createEnvelope, serializeEnvelope } from '@sentry/utils';
import { addItemToEnvelope, createAttachmentEnvelopeItem, createEnvelope, serializeEnvelope } from '@sentry/utils';
import * as http from 'http';
import { TextEncoder } from 'util';
import { createGunzip } from 'zlib';

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

Expand Down Expand Up @@ -34,17 +35,19 @@ let testServer: http.Server | undefined;

function setupTestServer(
options: TestServerOptions,
requestInspector?: (req: http.IncomingMessage, body: string) => void,
requestInspector?: (req: http.IncomingMessage, body: string, raw: Uint8Array) => void,
) {
testServer = http.createServer((req, res) => {
let body = '';
const chunks: Buffer[] = [];

req.on('data', data => {
body += data;
const stream = req.headers['content-encoding'] === 'gzip' ? req.pipe(createGunzip({})) : req;

stream.on('data', data => {
chunks.push(data);
});

req.on('end', () => {
requestInspector?.(req, body);
stream.on('end', () => {
requestInspector?.(req, chunks.join(), Buffer.concat(chunks));
});

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

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

const ATTACHMENT_ITEM = createAttachmentEnvelopeItem(
{ filename: 'empty-file.bin', data: new Uint8Array(50_000) },
new TextEncoder(),
);
const EVENT_ATTACHMENT_ENVELOPE = addItemToEnvelope(EVENT_ENVELOPE, ATTACHMENT_ITEM);
const SERIALIZED_EVENT_ATTACHMENT_ENVELOPE = serializeEnvelope(
EVENT_ATTACHMENT_ENVELOPE,
new TextEncoder(),
) as Uint8Array;

const defaultOptions = {
url: TEST_SERVER_URL,
recordDroppedEvent: () => undefined,
Expand Down Expand Up @@ -155,6 +168,40 @@ describe('makeNewHttpTransport()', () => {
});
});

describe('compression', () => {
it('small envelopes should not be compressed', async () => {
await setupTestServer(
{
statusCode: SUCCESS,
responseHeaders: {},
},
(req, body) => {
expect(req.headers['content-encoding']).toBeUndefined();
expect(body).toBe(SERIALIZED_EVENT_ENVELOPE);
},
);

const transport = makeNodeTransport(defaultOptions);
await transport.send(EVENT_ENVELOPE);
});

it('large envelopes should be compressed', async () => {
await setupTestServer(
{
statusCode: SUCCESS,
responseHeaders: {},
},
(req, _, raw) => {
expect(req.headers['content-encoding']).toEqual('gzip');
expect(raw.buffer).toStrictEqual(SERIALIZED_EVENT_ATTACHMENT_ENVELOPE.buffer);
},
);

const transport = makeNodeTransport(defaultOptions);
await transport.send(EVENT_ATTACHMENT_ENVELOPE);
});
});

describe('proxy', () => {
it('can be configured through option', () => {
makeNodeTransport({
Expand Down Expand Up @@ -236,104 +283,106 @@ describe('makeNewHttpTransport()', () => {
});
});

it('should register TransportRequestExecutor that returns the correct object from server response (rate limit)', async () => {
await setupTestServer({
statusCode: RATE_LIMIT,
responseHeaders: {},
});

makeNodeTransport(defaultOptions);
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];

const executorResult = registeredRequestExecutor({
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
category: 'error',
});

await expect(executorResult).resolves.toEqual(
expect.objectContaining({
describe('should register TransportRequestExecutor that returns the correct object from server response', () => {
it('rate limit', async () => {
await setupTestServer({
statusCode: RATE_LIMIT,
}),
);
});
responseHeaders: {},
});

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

makeNodeTransport(defaultOptions);
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
const executorResult = registeredRequestExecutor({
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
category: 'error',
});

const executorResult = registeredRequestExecutor({
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
category: 'error',
await expect(executorResult).resolves.toEqual(
expect.objectContaining({
statusCode: RATE_LIMIT,
}),
);
});

await expect(executorResult).resolves.toEqual(
expect.objectContaining({
it('OK', async () => {
await setupTestServer({
statusCode: SUCCESS,
headers: {
'retry-after': null,
'x-sentry-rate-limits': null,
},
}),
);
});
});

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

makeNodeTransport(defaultOptions);
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
const executorResult = registeredRequestExecutor({
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
category: 'error',
});

const executorResult = registeredRequestExecutor({
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
category: 'error',
await expect(executorResult).resolves.toEqual(
expect.objectContaining({
statusCode: SUCCESS,
headers: {
'retry-after': null,
'x-sentry-rate-limits': null,
},
}),
);
});

await expect(executorResult).resolves.toEqual(
expect.objectContaining({
it('OK with rate-limit headers', async () => {
await setupTestServer({
statusCode: SUCCESS,
headers: {
'retry-after': '2700',
'x-sentry-rate-limits': '60::organization, 2700::organization',
responseHeaders: {
'Retry-After': '2700',
'X-Sentry-Rate-Limits': '60::organization, 2700::organization',
},
}),
);
});
});

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

makeNodeTransport(defaultOptions);
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
const executorResult = registeredRequestExecutor({
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
category: 'error',
});

const executorResult = registeredRequestExecutor({
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
category: 'error',
await expect(executorResult).resolves.toEqual(
expect.objectContaining({
statusCode: SUCCESS,
headers: {
'retry-after': '2700',
'x-sentry-rate-limits': '60::organization, 2700::organization',
},
}),
);
});

await expect(executorResult).resolves.toEqual(
expect.objectContaining({
it('NOK with rate-limit headers', async () => {
await setupTestServer({
statusCode: RATE_LIMIT,
headers: {
'retry-after': '2700',
'x-sentry-rate-limits': '60::organization, 2700::organization',
responseHeaders: {
'Retry-After': '2700',
'X-Sentry-Rate-Limits': '60::organization, 2700::organization',
},
}),
);
});

makeNodeTransport(defaultOptions);
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];

const executorResult = registeredRequestExecutor({
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
category: 'error',
});

await expect(executorResult).resolves.toEqual(
expect.objectContaining({
statusCode: RATE_LIMIT,
headers: {
'retry-after': '2700',
'x-sentry-rate-limits': '60::organization, 2700::organization',
},
}),
);
});
});
});