Fix memory leak

[silesky]

The PriorityQueue class keeps track of any events (context) it sees so it can calculate whether an event can still be retried and how long the retry backoff should be.

We don’t clear an event’s entry from this map after it has been delivered, so overtime this map can get large.

Retries typically occur when there’s a network issue. For browsers, the memory leak is hard to detect because the in-memory map is cleared whenever the user navigates to a new page, and the persisted (localStorage) map has storage limits.

In node.js, an application can run for a long time, so the memory leak is more likely to build up over time.

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 7486f06e48f721dce9ae96530427ade3ec134882

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[chrisradek]

The calculateMaxTotalRetryTime calculation makes sense - it's the sum of the maximum backoff amounts given an allowed number of attempts.

I think this leads to a lower expiration than expected though.

• The expiration doesn't take into account time spent actually processing an event (going through plugins for example)
• The persisted queue will persist any unsent events to localstorage when the user navigates away from the page. When the user navigates to a page in the same domain, those events sent. This is the riskier scenario as it becomes much easier to hit the expiration in this case.

I haven't thought this all the way through but I wonder if we can move the attempts data to the Context. We already have attempts stored there for the EventQueue - the priority queue is (I think) meant to track attempts per destination (each destination has its own priority queue).

[silesky]

• The expiration doesn't take into account time spent actually processing an event (going through plugins for example)

I didn't consider this -- can we add a cushion of a minute? 5 minutes?

• The persisted queue will persist any unsent events to localstorage when the user navigates away from the page. When the user navigates to a page in the same domain, those events sent. This is the riskier scenario as it becomes much easier to hit the expiration in this case.

Thanks for flagging this! Good find -- I didn't think about the browser scenario since memory leaks haven't been a concern there, but it'd be great to also find a solution there. I do see a way around this -- to allow resumes, on instantiation of the queue, we could store the queueInitTime as a field on the instance and add it as a property to each event (think of the queueInitTime kind of like a "sessionID" ). Then, we can check on push if that event has an old queueInitTime, and If so, we know its from localStorage, and we update that event with the new expiration (newExpiration = this.queueInitTime + event.expiration - event.queueInitTime).

We already have attempts stored there for the EventQueue - the priority queue is (I think) meant to track attempts per destination (each destination has its own priority queue).

Hmm, I don't 100% understand this -- I only see EventQueue initialized once, on load. Maybe we can talk offline?