-
Notifications
You must be signed in to change notification settings - Fork 89
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
Fix queue processing #2525
Fix queue processing #2525
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
const success = await this.taskQueue.push(task) | ||
if (!success) { | ||
this.logger(`Will try to re-attempt task ${task.taskId} with data ${task.data}`) | ||
await this.taskQueue.push({ ...task, tries: task.tries + 1 }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to only retry once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we would retry differently depending on the error but figuring out all of the errors that are possible with libp2p that amount to "we couldn't connect because we haven't been able to yet" or "this peer is offline and we won't connect right now no matter what" to filter those out and only retry multiple times when its something that would keep us from adding the peer to the peerstore and auto-dialing.
I don't think it's worth doing that in this PR but we should be doing it.
* Process queue fixes and updates * Remove unnecessary log * Log when we push to queue
Pull Request Checklist
(Optional) Mobile checklist
Please ensure you completed the following checks if you did any changes to the mobile package: