feat: Offline Integration #2216
Conversation
To handle errors occuring offline in JavaScript this integration saves them in a queue and processes these queued up errors when back online. As a storage API localForage (https://github.com/localForage/localForage) is used. A new function listens to the 'online' event to start draining the queue. I am missing correct error handling and tests. Also I am not sure about the correct way to include the interation to sdk.ts. Building .min and .es6 files is also not working with 'yarn build'. Minifing with uglify-js works. Thanks especially to @kamilogorek and @ seromenho Resolves: getsentry#1633 See also: getsentry#1665 getsentry#279
|
Thanks for tackling this! That's a very good start! Can you move this integration to |
| import { Event, Integration } from '@sentry/types'; | ||
| import localforage from 'localforage'; | ||
|
|
||
| import { captureEvent } from '../index'; |
There was a problem hiding this comment.
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
| /** | ||
| * the key to store the offline event queue | ||
| */ | ||
| private readonly _storrageKey: string = 'offlineEventStore'; |
| addGlobalEventProcessor(async (event: Event) => { | ||
| const self = getCurrentHub().getIntegration(Offline); | ||
| if (self) { | ||
| if (navigator.onLine) { |
There was a problem hiding this comment.
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.
| } | ||
| await this._storeEvent(event); | ||
| return null; | ||
| // self._storeEvent(event); |
| * @param event an event | ||
| */ | ||
| private async _storeEvent(event: Event): Promise<void> { | ||
| const storrageKey = this._storrageKey; |
| const promise: Promise<void> = new Promise(async function(resolve: () => void, reject: () => void): Promise<void> { | ||
| let queue: Event[] = []; | ||
| // get queue | ||
| const value = await offlineEventStore.getItem(storrageKey).catch(function(): void { |
There was a problem hiding this comment.
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') { |
There was a problem hiding this comment.
Same comments as in the storage part
| } | ||
| await offlineEventStore.removeItem(storrageKey).catch(function(): void { | ||
| // could not remove queue from localForge | ||
| reject(); |
There was a problem hiding this comment.
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') { |
There was a problem hiding this comment.
Nothing bad should happen if you pass undefined to captureEvent anyway
| @@ -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'; | |||
There was a problem hiding this comment.
It should be pluggable integration for sure. It's too heavy for the core.
kamilogorek
changed the title
|
Hello, I would like to integrate Sentry in a mobile application (Ionic), but without Offline mode, this lib loses a little interest. Can you tell me if the offline mode will be integrated soon? |
|
@fcastell sorry but I'm not able to give you any specific dates at this time. |
|
Hi. Will this feature get integrated anytime soon? it is very needed for my application. Best regards |
|
It's not planned anytime soon, sorry. This PR is in a great state though if someone wants to pick it up and take to the finish line :) |
|
@kamilogorek is it just the recommended changes listed above that need done? |
|
Closing this PR in favor of #2778 Thank you! |
To handle errors occuring offline in JavaScript this integration saves
them in a queue and processes these queued up errors when back online.
As a storage API localForage (https://github.com/localForage/localForage)
is used. A new function listens to the 'online' event to start draining
the queue.
I am missing correct error handling and tests. Also I am not sure about
the correct way to include the interation to sdk.ts.
Building .min and .es6 files is also not working with 'yarn build'.
Minifing with uglify-js works.
Thanks especially to @kamilogorek and @ seromenho
Resolves: #1633
See also: #1665 #279