Skip to content

feat: Offline Integration #2216

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

Closed
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
@@ -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",
1 change: 1 addition & 0 deletions packages/browser/src/integrations/index.ts
Original file line number Diff line number Diff line change
@@ -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
@@ -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(),
@@ -14,6 +14,7 @@ export const defaultIntegrations = [
new GlobalHandlers(),
new LinkedErrors(),
new UserAgent(),
new Offline(),
];

/**
8 changes: 6 additions & 2 deletions packages/browser/tsconfig.build.json
Original file line number Diff line number Diff line change
@@ -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
@@ -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
@@ -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
@@ -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"
@@ -6940,6 +6945,13 @@ libnpmpublish@^1.1.1:
semver "^5.5.1"
ssri "^6.0.1"

[email protected]:
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"
@@ -6973,6 +6985,13 @@ loader-utils@^1.1.0:
emojis-list "^2.0.0"
json5 "^0.5.0"

[email protected]:
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"