Skip to content

feat: Add simple transport buffer with close function #1504

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 9 commits into from
Sep 3, 2018
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
18 changes: 14 additions & 4 deletions packages/browser/src/backend.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Backend, logger, Options, SentryError } from '@sentry/core';
import { Backend, logger, Options, SentryError, TransportBuffer } from '@sentry/core';
import { SentryEvent, SentryEventHint, SentryResponse, Severity, Status, Transport } from '@sentry/types';
import { isDOMError, isDOMException, isError, isErrorEvent, isPlainObject } from '@sentry/utils/is';
import { supportsBeacon, supportsFetch } from '@sentry/utils/supports';
Expand Down Expand Up @@ -40,6 +40,9 @@ export class BrowserBackend implements Backend {
/** Cached transport used internally. */
private transport?: Transport;

/** A simple buffer holding all requests. */
private readonly buffer: TransportBuffer<SentryResponse> = new TransportBuffer();

/**
* @inheritDoc
*/
Expand Down Expand Up @@ -141,8 +144,8 @@ export class BrowserBackend implements Backend {
public async sendEvent(event: SentryEvent): Promise<SentryResponse> {
if (!this.options.dsn) {
logger.warn(`Event has been skipped because no Dsn is configured.`);
// We do nothing in case there is no Dsn
return { status: Status.Skipped };
// We do nothing in case there is no DSN
return { status: Status.Skipped, reason: `Event has been skipped because no Dsn is configured.` };
}

if (!this.transport) {
Expand All @@ -161,7 +164,7 @@ export class BrowserBackend implements Backend {
}
}

return this.transport.send(event);
return this.buffer.add(this.transport.captureEvent(event));
}

/**
Expand All @@ -177,4 +180,11 @@ export class BrowserBackend implements Backend {
public storeScope(): void {
// Noop
}

/**
* @inheritDoc
*/
public async close(timeout?: number): Promise<boolean> {
return this.buffer.drain(timeout);
}
}
4 changes: 2 additions & 2 deletions packages/browser/src/transports/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export abstract class BaseTransport implements Transport {
/**
* @inheritDoc
*/
public async send(_: SentryEvent): Promise<SentryResponse> {
throw new SentryError('Transport Class has to implement `send` method');
public async captureEvent(_: SentryEvent): Promise<SentryResponse> {
throw new SentryError('Transport Class has to implement `captureEvent` method');
}
}
2 changes: 1 addition & 1 deletion packages/browser/src/transports/beacon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export class BeaconTransport extends BaseTransport {
/**
* @inheritDoc
*/
public async send(event: SentryEvent): Promise<SentryResponse> {
public async captureEvent(event: SentryEvent): Promise<SentryResponse> {
const data = serialize(event);

const result = global.navigator.sendBeacon(this.url, data);
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/transports/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export class FetchTransport extends BaseTransport {
/**
* @inheritDoc
*/
public async send(event: SentryEvent): Promise<SentryResponse> {
public async captureEvent(event: SentryEvent): Promise<SentryResponse> {
const defaultOptions: RequestInit = {
body: serialize(event),
method: 'POST',
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/transports/xhr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export class XHRTransport extends BaseTransport {
/**
* @inheritDoc
*/
public async send(event: SentryEvent): Promise<SentryResponse> {
public async captureEvent(event: SentryEvent): Promise<SentryResponse> {
return new Promise<SentryResponse>((resolve, reject) => {
const request = new XMLHttpRequest();

Expand Down
4 changes: 2 additions & 2 deletions packages/browser/test/backend.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { BrowserBackend } from '../src/backend';
import { BaseTransport } from '../src/transports';

class SimpleTransport extends BaseTransport {
public async send(_: SentryEvent): Promise<SentryResponse> {
public async captureEvent(_: SentryEvent): Promise<SentryResponse> {
return {
status: Status.fromHttpCode(200),
};
Expand Down Expand Up @@ -34,7 +34,7 @@ describe('BrowserBackend', () => {
}
});

it('should call send() on provided transport', async () => {
it('should call captureEvent() on provided transport', async () => {
backend = new BrowserBackend({ dsn, transport: SimpleTransport });
const status = await backend.sendEvent(testEvent);
expect(status.status).equal(Status.Success);
Expand Down
12 changes: 9 additions & 3 deletions packages/browser/test/integration/frame.html
Original file line number Diff line number Diff line change
Expand Up @@ -110,19 +110,25 @@

// stub transport so we don't actually transmit any data
function DummyTransport() { }
DummyTransport.prototype.send = function (event) {
DummyTransport.prototype.captureEvent = function (event) {
// console.log(JSON.stringify(event, null, 2));
sentryData.push(event);
return {
return Promise.resolve({
status: 'success'
}
})
}

Sentry.init({
dsn: 'https://[email protected]/1',
attachStacktrace: true,
transport: DummyTransport,
beforeBreadcrumb: function (breadcrumb) {
// Filter console logs as we use them for debugging *a lot* and they are not *that* important
if (breadcrumb.category === 'console') { return null }

// overlyComplicatedDebuggingMechanism 'aka' console.log driven debugging
// console.log(JSON.stringify(breadcrumb, null, 2));

// Filter internal Karma requests
if (
breadcrumb.type === 'http' &&
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/test/integration/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ describe('integration', function() {
done,
function() {
var xhr = new XMLHttpRequest();
xhr.open('GET', 'https://public@example.com/1/store');
xhr.open('GET', 'https://example.com/api/1/store/');
xhr.send('{"message":"someMessage","level":"warning"}');
setTimeout(done);
},
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/test/transports/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ const testDsn = 'https://[email protected]/42';
class SimpleTransport extends BaseTransport {}

describe('BaseTransport', () => {
it('doesnt provide send() implementation', async () => {
it('doesnt provide captureEvent() implementation', async () => {
const transport = new SimpleTransport({ dsn: testDsn });

try {
await transport.send({});
await transport.captureEvent({});
} catch (e) {
expect(e.message).equal('Transport Class has to implement `send` method');
expect(e.message).equal('Transport Class has to implement `captureEvent` method');
}
});

Expand Down
6 changes: 3 additions & 3 deletions packages/browser/test/transports/beacon.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ describe('BeaconTransport', () => {
expect(transport.url).equal(transportUrl);
});

describe('send()', async () => {
describe('captureEvent()', async () => {
it('sends a request to Sentry servers', async () => {
sendBeacon.returns(true);

return transport.send(payload).then(res => {
return transport.captureEvent(payload).then(res => {
expect(res.status).equal(Status.Success);
expect(sendBeacon.calledOnce).equal(true);
expect(sendBeacon.calledWith(transportUrl, JSON.stringify(payload))).equal(true);
Expand All @@ -43,7 +43,7 @@ describe('BeaconTransport', () => {
it('rejects with failed status', async () => {
sendBeacon.returns(false);

return transport.send(payload).catch(res => {
return transport.captureEvent(payload).catch(res => {
expect(res.status).equal(Status.Failed);
expect(sendBeacon.calledOnce).equal(true);
expect(sendBeacon.calledWith(transportUrl, JSON.stringify(payload))).equal(true);
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/test/transports/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ describe('FetchTransport', () => {
expect(transport.url).equal(transportUrl);
});

describe('send()', async () => {
describe('captureEvent()', async () => {
it('sends a request to Sentry servers', async () => {
const response = { status: 200 };

fetch.returns(Promise.resolve(response));

return transport.send(payload).then(res => {
return transport.captureEvent(payload).then(res => {
expect(res.status).equal(Status.Success);
expect(fetch.calledOnce).equal(true);
expect(
Expand All @@ -53,7 +53,7 @@ describe('FetchTransport', () => {

fetch.returns(Promise.reject(response));

return transport.send(payload).catch(res => {
return transport.captureEvent(payload).catch(res => {
expect(res.status).equal(403);
expect(fetch.calledOnce).equal(true);
expect(
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/test/transports/xhr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ describe('XHRTransport', () => {
expect(transport.url).equal(transportUrl);
});

describe('send()', async () => {
describe('captureEvent()', async () => {
it('sends a request to Sentry servers', async () => {
server.respondWith('POST', transportUrl, [200, {}, '']);

return transport.send(payload).then(res => {
return transport.captureEvent(payload).then(res => {
expect(res.status).equal(Status.Success);
const request = server.requests[0];
expect(server.requests.length).equal(1);
Expand All @@ -46,7 +46,7 @@ describe('XHRTransport', () => {
it('rejects with non-200 status code', done => {
server.respondWith('POST', transportUrl, [403, {}, '']);

transport.send(payload).catch(res => {
transport.captureEvent(payload).catch(res => {
expect(res.status).equal(403);

const request = server.requests[0];
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,4 +310,11 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement

return response;
}

/**
* @inheritDoc
*/
public async close(timeout?: number): Promise<boolean> {
return this.backend.close(timeout);
}
}
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ export { API } from './api';
export { BackendClass, BaseClient } from './base';
export { Dsn } from './dsn';
export { SentryError } from './error';
export { TransportBuffer } from './transportbuffer';
export { Backend, Client, LogLevel, Options } from './interfaces';
export { initAndBind, ClientClass } from './sdk';
17 changes: 17 additions & 0 deletions packages/core/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,15 @@ export interface Client<O extends Options = Options> {

/** Returns the current options. */
getOptions(): O;

/**
* A promise that resolves whenever the transport buffer is empty.
* If you provide a timeout and the buffer takes longer to drain the promise returns false.
* Calls {@link Backend.close}.
*
* @param timeout Maximum time in ms the client should wait.
*/
close(timeout?: number): Promise<boolean>;
}

/**
Expand Down Expand Up @@ -254,4 +263,12 @@ export interface Backend {
* @param scope The scope to store.
*/
storeScope(scope: Scope): void;

/**
* A promise that resolves whenever the transport buffer is empty.
* If you provide a timeout and the buffer takes longer to drain the promise returns false.
*
* @param timeout Maximum time in ms the client should wait.
*/
close(timeout?: number): Promise<boolean>;
}
48 changes: 48 additions & 0 deletions packages/core/src/transportbuffer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/** A simple queue that holds promises. */
export class TransportBuffer<T> {
/** Internal set of queued Promises */
private readonly buffer: Set<Promise<T>> = new Set();

/**
* Add a promise to the queue.
*
* @param task Can be any Promise<T>
* @returns The original promise.
*/
public async add(task: Promise<T>): Promise<T> {
this.buffer.add(task);
task.then(() => this.buffer.delete(task)).catch(() => this.buffer.delete(task));
return task;
}

/**
* This function returns the number of unresolved promises in the queue.
*/
public length(): number {
return this.buffer.size;
}

/**
* This will drain the whole queue, returns true if queue is empty or drained.
* If timeout is provided and the queue takes longer to drain, the promise still resolves but with false.
*
* @param timeout Number in ms to wait until it resolves with false.
*/
public async drain(timeout?: number): Promise<boolean> {
return new Promise<boolean>(resolve => {
const capturedSetTimeout = setTimeout(() => {
if (timeout && timeout > 0) {
resolve(false);
}
}, timeout);
Promise.all(this.buffer.values())
.then(() => {
clearTimeout(capturedSetTimeout);
resolve(true);
})
.catch(() => {
resolve(true);
});
});
}
}
Loading