Skip to content
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

#25 Emit added for queue overflow. Tests added as well #26

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Dahaden
Copy link
Contributor

@Dahaden Dahaden commented Nov 13, 2020

Adds support for emits recording events dropped from the queue due to overflow

lib/index.js Outdated
@@ -170,8 +170,7 @@ Queue.prototype.requeue = function(item, attemptNumber, error) {

Queue.prototype._enqueue = function(entry) {
var queue = this._store.get(this.keys.QUEUE) || [];
queue = queue.slice(-(this.maxItems - 1));
queue.push(entry);
var removedEntrys = queue.splice(0, queue.length - (this.maxItems - 1), entry);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice

If the 2nd arg is negative or 0, there will be no items deleted

Also, Im not sure on what the minimum supported version of IE is, but Splice is IE5.5 as opposed to IE4 for Slice
They are extremely old but thought I should do my due diligence :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add these comments to the code for future visibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do that now :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call on this, realized past me made a mistake in reading the original implementation thinking that we deleted events at the start of the queue instead of the end.

Have updated this and added more comments :)

@Dahaden Dahaden mentioned this pull request Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants