Offline storage plugin #1165

seromenho · 2017-12-04T16:08:42Z

This is my proposal to #279. Any thoughts/suggestions to improve or make it different are welcome.
Thanks.
(#973 continues here.)

Description

Stores errors failed to get send and try to send them when network comes back or on init

Options

onlineEventName: In case you have a custom event fired instead of online you can pass it here.

TODO

If this works for you I'm missing documentation and tests.

Stores errors failed to get send and try to send them when Network comes back or on init

kamilogorek · 2017-12-05T11:28:42Z

plugins/offline-storage.js

+
+var offlineStorageKey = 'raven-js-offline-queue';
+
+function offlineStoragelugin(Raven, options) {


Type in offlineStoragelugin

kamilogorek · 2017-12-05T11:28:50Z

plugins/offline-storage.js

+  window.addEventListener(options.onlineEventName || 'online', processOfflineQueue);
+}
+
+module.exports = offlineStoragelugin;


Typo here as well

kamilogorek · 2017-12-05T11:29:53Z

plugins/offline-storage.js

+      var queue = JSON.parse(localStorage.getItem(offlineStorageKey)) || [];
+
+      // Store an empty queue. If processing these one fails they get back to the queue
+      localStorage.setItem(offlineStorageKey, JSON.stringify([]));


We can call removeItem instead to be more direct. It'll fallback to [] anyway during processing

kamilogorek · 2017-12-05T11:33:08Z

plugins/offline-storage.js

+      queue.forEach(function processOfflinePayload(data) {
+        // Avoid duplication verification for offline stored
+        // as they may try multiple times to be processed
+        Raven._lastData = null;


I'm not sure if I understand your comment here.
There shouldn't be any duplicates here already, as failure event is triggered at the end of _sendProcessedPayload which already checks for duplicates first.

Yes, but then when trying to send it again when get back offline, because _lastData is set with potentially the same data we will be try to send, it will be missed because it's the same data.

kamilogorek · 2017-12-05T11:34:17Z

plugins/offline-storage.js

+
+    try {
+      var queue = JSON.parse(localStorage.getItem(offlineStorageKey)) || [];
+      queue.push(event.data || Raven._lastData);


What's the purpose of falling back to _lastData?

Just to make sure, from what I have seen they will have(and be) to be the same right?
I can remove the fallback.

kamilogorek · 2017-12-05T11:35:16Z

plugins/offline-storage.js

+  });
+
+  // Add event listener on online or custom event to trigger offline queue sending
+  window.addEventListener(options.onlineEventName || 'online', processOfflineQueue);


Is there any alternative event that I don't know of, but could be used here? :P

This is just because the application might rely on a different event for letting other plugins know when the connection is available (e.g. on our cordova-based hybrid app we trigger a "foo-bar-online" event to the document when we're sure the app is online and it was able to make an XHR request), because "online" can easily mean "connected to the network" as in "local network" but still not be connected to the internet.

Makes perfect sense

Based on PR review

seromenho · 2017-12-05T12:53:15Z

@kamilogorek Thank you som much for your review!
I have implemented your requested changes.
Let me know if it's all good now.

kamilogorek · 2017-12-05T14:00:16Z

I'd say the implementation is fine now, thanks! :)
Would you mind adding some tests for the main functionality?

seromenho · 2017-12-05T16:38:54Z

Great, thanks.
As far for the tests. I won't be able to add them at least on this week.
I'll try to add them when possible or I'll accept help from anyone 😄

kamilogorek · 2017-12-06T09:45:49Z

No rush, take your time :)
Unfortunately, I have some other tasks to tackle, so I cannot help this week as well.

taylorhakes · 2017-12-26T03:47:48Z

plugins/offline-storage.js

+      var queue = JSON.parse(localStorage.getItem(offlineStorageKey)) || [];
+
+      // Store an empty queue. If processing these one fails they get back to the queue
+      localStorage.removeItem(offlineStorageKey);


multiple localStorage operations are not atomic. It is possible for multiple tabs to receive this queue before the removeItem. This needs to be inside a lock. See more info here:
https://medium.engineering/wait-dont-touch-that-a211832adc3a
http://balpha.de/2012/03/javascript-concurrency-and-locking-the-html5-localstorage/

https://www.npmjs.com/package/localstorage-fifo ?

@graingert I don't believe that library handles atomic operations between multiple browser windows. It just removes oldest item when localstorage when it's full

Here is a link to a library I created to solve this problem. There are probably other libraries out there as well. Also feel free to just copy the library code into this repo if you don't want another dependency.

https://github.com/taylorhakes/localstorage-lock/

Storing an entire array of the logs at once can be quite a poor usage of memory. There might be a lot of logs if a device is left offline for extended periods of time. On hybrid apps there is a very small memory limit. If there are a lot of logs or large json encoded objects, memory can spike. Possibly using indexdb or an option for a custom storage adapter would allow for more memory efficient ways of storing.

cvan · 2018-02-26T23:47:15Z

plugins/offline-storage.js

+ * Offline Storage plugin
+ * 
+ * Stores errors failed to get send and try to send them when
+ * Networkf comes back or on init


(sorry, this is a drive-by comment)

is this a typo, Networkf? should be Network, right?

cvan · 2018-02-27T01:16:58Z

plugins/offline-storage.js

+/**
+ * Offline Storage plugin
+ * 
+ * Stores errors failed to get send and try to send them when


should failed to get send be that failed to be sent?

kamilogorek · 2018-08-02T09:57:59Z

Not relevant for sentry-javascript SDK v4.

AgDude · 2018-08-02T12:55:23Z

@kamilogorek I am excited to hear that apparently raven will support offline natively. Any estimate on when we can expect to see v4?

kamilogorek · 2018-08-02T13:42:00Z

@AgDude Sentry core and node packages are already released as beta. We also merged major browser sdk rewrite (https://github.com/getsentry/sentry-javascript/commits/master quite a lot of commits :p) Today and now in a process of cleaning things outs.

v4 will be extensible in a lot of ways, therefore adding offline integration (we'll write it ourselves anyway) will be a breeze :)

It should be released as a major version early Q3, around September/October.

AgDude · 2018-08-02T16:38:31Z

Sounds great! Thanks for the update.

seromenho · 2018-08-02T16:40:20Z

Awesome! Thanks

joelryan2k · 2018-10-08T19:41:15Z

@kamilogorek We too are interested in capturing errors offline. Any update on this?

If someone can provide general direction on the recommended approach I have time to work on this... or test a work in progress. I see from your August 2nd comment that it appears this can be built as an Sentry 4 "Integration"?

kamilogorek · 2018-10-08T20:58:39Z

@joelryan2k veeeery rough sketch of what it can look like. We'll definitely release an official integration, but we still have a lot of things to polish first.

import { configureScope, captureEvent } from '@sentry/browser';
import { serialize, deserialize } from '@sentry/utils/object';

class Offline {
  constructor() {
    // initialize storage
    this.storage = someApi;
    // get items
    this.queue = this.storage.getEvents();
    // drain them
    this.drainQueue(this.queue);
  }
  
  install() {
    configureScope(scope => scope.addEventProcessor(async (event) => {
      if (isOnline()) {
        return event;
      }
      await this.storeEvent(event);
      return event;
    }))
  }
  
  storeEvent(event) {
    return this.storage.storeEvent(serialize(event));
  }
  
  drainQueue(queue) {
    // send each one to sentry
    queue.forEach((event) => captureEvent(deserialize(event)));
  }
}

kamilogorek · 2018-10-09T06:39:52Z

Created #1633 to keep a track of this feature

Offline storage plugin

ebf5ad2

Stores errors failed to get send and try to send them when Network comes back or on init

squallstar approved these changes Dec 4, 2017

View reviewed changes

kamilogorek suggested changes Dec 5, 2017

View reviewed changes

Typo on offline storage plugin and improvements

fcb8db2

Based on PR review

kamilogorek added Status: In Progress labels Dec 6, 2017

squallstar mentioned this pull request Dec 15, 2017

Errors on Offline app #973

Closed

taylorhakes reviewed Dec 26, 2017

View reviewed changes

kamilogorek mentioned this pull request Jan 18, 2018

Directions for offline reporting? #279

Closed

cvan reviewed Feb 26, 2018

View reviewed changes

cvan reviewed Feb 27, 2018

View reviewed changes

cvan mentioned this pull request Feb 27, 2018

add base Telemetry JavaScript snippet (fixes issue #172) MozillaReality/unity-webxr-export#174

Closed

kamilogorek closed this Aug 2, 2018

kamilogorek mentioned this pull request Oct 9, 2018

Offline Integration #1633

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Offline storage plugin #1165

Offline storage plugin #1165

seromenho commented Dec 4, 2017

kamilogorek Dec 5, 2017

kamilogorek Dec 5, 2017

seromenho Dec 5, 2017

kamilogorek Dec 5, 2017

seromenho Dec 5, 2017

kamilogorek Dec 5, 2017

seromenho Dec 5, 2017

kamilogorek Dec 5, 2017

kamilogorek Dec 5, 2017

seromenho Dec 5, 2017

kamilogorek Dec 5, 2017

seromenho Dec 5, 2017

kamilogorek Dec 5, 2017

seromenho commented Dec 5, 2017

kamilogorek commented Dec 5, 2017

seromenho commented Dec 5, 2017

kamilogorek commented Dec 6, 2017

taylorhakes Dec 26, 2017

graingert Feb 14, 2018

taylorhakes Feb 16, 2018

taylorhakes Feb 16, 2018

jasekiw Mar 16, 2018 •

edited

cvan Feb 26, 2018

cvan Feb 27, 2018

kamilogorek commented Aug 2, 2018

AgDude commented Aug 2, 2018

kamilogorek commented Aug 2, 2018

AgDude commented Aug 2, 2018

seromenho commented Aug 2, 2018

joelryan2k commented Oct 8, 2018

kamilogorek commented Oct 8, 2018 •

edited

kamilogorek commented Oct 9, 2018


		var offlineStorageKey = 'raven-js-offline-queue';

		function offlineStoragelugin(Raven, options) {

Offline storage plugin #1165

Offline storage plugin #1165

Conversation

seromenho commented Dec 4, 2017

Description

Options

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seromenho commented Dec 5, 2017

kamilogorek commented Dec 5, 2017

seromenho commented Dec 5, 2017

kamilogorek commented Dec 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasekiw Mar 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamilogorek commented Aug 2, 2018

AgDude commented Aug 2, 2018

kamilogorek commented Aug 2, 2018

AgDude commented Aug 2, 2018

seromenho commented Aug 2, 2018

joelryan2k commented Oct 8, 2018

kamilogorek commented Oct 8, 2018 • edited

kamilogorek commented Oct 9, 2018

jasekiw Mar 16, 2018 •

edited

kamilogorek commented Oct 8, 2018 •

edited