Skip to content
Closed
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
3 changes: 2 additions & 1 deletion packages/browser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
"@sentry/core": "5.6.2",
"@sentry/types": "5.6.1",
"@sentry/utils": "5.6.1",
"tslib": "^1.9.3"
"tslib": "^1.9.3",
"localforage": "1.7.3"
},
"devDependencies": {
"@types/md5": "2.1.33",
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/integrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export { TryCatch } from './trycatch';
export { Breadcrumbs } from './breadcrumbs';
export { LinkedErrors } from './linkederrors';
export { UserAgent } from './useragent';
export { Offline } from './offline';
116 changes: 116 additions & 0 deletions packages/browser/src/integrations/offline.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core';
import { Event, Integration } from '@sentry/types';
import localforage from 'localforage';

import { captureEvent } from '../index';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use captureEvent from @sentry/minimal. This way can make this integration work on node.js and on browser if necessary by using something like https://github.com/sindresorhus/is-online


/**
* store errors occuring offline and send them when online again
*/
export class Offline implements Integration {
/**
* @inheritDoc
*/
public readonly name: string = Offline.id;

/**
* @inheritDoc
*/
public static id: string = 'Offline';

/**
* the key to store the offline event queue
*/
private readonly _storrageKey: string = 'offlineEventStore';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo stor**R**age


public offlineEventStore: LocalForage;

/**
* @inheritDoc
*/
public setupOnce(): void {
addGlobalEventProcessor(async (event: Event) => {
const self = getCurrentHub().getIntegration(Offline);
if (self) {
if (navigator.onLine) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break on environments that may miss this API (eg. some testing environments, or some odd devices (we had those in the paste)).
Can you use getGlobalObject from our @sentry/utils and check if navigator and onLine are there first? eg. 'onLine' in navigator.

return event;
}
await this._storeEvent(event);
return null;
// self._storeEvent(event);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment

}
return event;
});
}

public constructor() {
this.offlineEventStore = localforage.createInstance({
name: 'sentryOfflineEventStore',
});
window.addEventListener('online', () => {
this._drainQueue().catch(function(): void {
// TODO: handle rejected promise
});
});
}

/**
* store an event
* @param event an event
*/
private async _storeEvent(event: Event): Promise<void> {
const storrageKey = this._storrageKey;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

const offlineEventStore = this.offlineEventStore;
const promise: Promise<void> = new Promise(async function(resolve: () => void, reject: () => void): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can return promise directly here

let queue: Event[] = [];
const value = await offlineEventStore.getItem(storrageKey);
// .then(function(value: unknown): void {
// })
// .catch(function(err: Error): void {
// console.log(err);
// });
if (typeof value === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always try/catch around JSON.parse, as who knows when it can break.

queue = JSON.parse(value);
}
queue.push(event);
await offlineEventStore.setItem(storrageKey, JSON.stringify(queue)).catch(function(): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use normalize from @sentry/utils to preserve more data than JSON.stringify does.

// reject promise because saving to the localForge store did not work
reject();
});
resolve();
});
return promise;
}

/**
* capture all events in the queue
*/
private async _drainQueue(): Promise<void> {
const storrageKey = this._storrageKey;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

const offlineEventStore = this.offlineEventStore;
const promise: Promise<void> = new Promise(async function(resolve: () => void, reject: () => void): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, can return directly

let queue: Event[] = [];
// get queue
const value = await offlineEventStore.getItem(storrageKey).catch(function(): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can uselogger.warn('could not read the queue for offline integration'); from @sentry/utils

// could not get queue from localForge, TODO: how to handle error?
});
// TODO: check if value in localForge can be converted with JSON.parse
if (typeof value === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments as in the storage part

queue = JSON.parse(value);
}
await offlineEventStore.removeItem(storrageKey).catch(function(): void {
// could not remove queue from localForge
reject();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass reject as the argument to catch directly instead of wrapping it in another function

});
// process all events in the queue
while (queue.length > 0) {
const event = queue.pop();
if (typeof event !== 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing bad should happen if you pass undefined to captureEvent anyway

captureEvent(event);
}
}
resolve();
});
return promise;
}
}
3 changes: 2 additions & 1 deletion packages/browser/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { getGlobalObject } from '@sentry/utils';
import { BrowserOptions } from './backend';
import { BrowserClient, ReportDialogOptions } from './client';
import { wrap as internalWrap } from './helpers';
import { Breadcrumbs, GlobalHandlers, LinkedErrors, TryCatch, UserAgent } from './integrations';
import { Breadcrumbs, GlobalHandlers, LinkedErrors, Offline, TryCatch, UserAgent } from './integrations';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be pluggable integration for sure. It's too heavy for the core.


export const defaultIntegrations = [
new CoreIntegrations.InboundFilters(),
Expand All @@ -14,6 +14,7 @@ export const defaultIntegrations = [
new GlobalHandlers(),
new LinkedErrors(),
new UserAgent(),
new Offline(),
];

/**
Expand Down
8 changes: 6 additions & 2 deletions packages/browser/tsconfig.build.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
"extends": "../../tsconfig.json",
"compilerOptions": {
"baseUrl": ".",
"outDir": "dist"
"outDir": "./dist"
},
"include": ["src/**/*"]
"include": ["src/**/*"],
"exclude": [
"build/**/*",
"dist/**/*"
]
}
4 changes: 3 additions & 1 deletion packages/browser/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
"exclude": ["dist"],
"compilerOptions": {
"rootDir": ".",
"types": ["node", "mocha", "chai", "sinon"]
"types": ["node", "mocha", "chai", "sinon"],
"esModuleInterop": true,
"allowSyntheticDefaultImports": true
}
}
4 changes: 3 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
"@sentry/*": ["*/src"],
"raven-js": ["raven-js/src/singleton.js"],
"raven-node": ["raven-node/lib/client.js"]
}
},
"allowSyntheticDefaultImports": true,
"esModuleInterop": true
}
}
19 changes: 19 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5490,6 +5490,11 @@ ignore@^3.3.5:
resolved "https://registry.yarnpkg.com/ignore/-/ignore-3.3.10.tgz#0a97fb876986e8081c631160f8f9f389157f0043"
integrity sha512-Pgs951kaMm5GXP7MOvxERINe3gsaVjUWFm+UZPSq9xYriQAksyhg0csnS0KXSNRD5NmNdapXEpjxG49+AKh/ug==

immediate@~3.0.5:
version "3.0.6"
resolved "https://registry.yarnpkg.com/immediate/-/immediate-3.0.6.tgz#9db1dbd0faf8de6fbe0f5dd5e56bb606280de69b"
integrity sha1-nbHb0Pr43m++D13V5Wu2BigN5ps=

import-fresh@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/import-fresh/-/import-fresh-2.0.0.tgz#d81355c15612d386c61f9ddd3922d4304822a546"
Expand Down Expand Up @@ -6940,6 +6945,13 @@ libnpmpublish@^1.1.1:
semver "^5.5.1"
ssri "^6.0.1"

lie@3.1.1:
version "3.1.1"
resolved "https://registry.yarnpkg.com/lie/-/lie-3.1.1.tgz#9a436b2cc7746ca59de7a41fa469b3efb76bd87e"
integrity sha1-mkNrLMd0bKWd56QfpGmz77dr2H4=
dependencies:
immediate "~3.0.5"

load-json-file@^1.0.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/load-json-file/-/load-json-file-1.1.0.tgz#956905708d58b4bab4c2261b04f59f31c99374c0"
Expand Down Expand Up @@ -6973,6 +6985,13 @@ loader-utils@^1.1.0:
emojis-list "^2.0.0"
json5 "^0.5.0"

localforage@1.7.3:
version "1.7.3"
resolved "https://registry.yarnpkg.com/localforage/-/localforage-1.7.3.tgz#0082b3ca9734679e1bd534995bdd3b24cf10f204"
integrity sha512-1TulyYfc4udS7ECSBT2vwJksWbkwwTX8BzeUIiq8Y07Riy7bDAAnxDaPU/tWyOVmQAcWJIEIFP9lPfBGqVoPgQ==
dependencies:
lie "3.1.1"

locate-path@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/locate-path/-/locate-path-2.0.0.tgz#2b568b265eec944c6d9c0de9c3dbbbca0354cd8e"
Expand Down