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

Fix queue processing #2525

Merged
merged 3 commits into from
May 13, 2024
Merged

Fix queue processing #2525

merged 3 commits into from
May 13, 2024

Conversation

islathehut
Copy link
Collaborator

Pull Request Checklist

  • I have linked this PR to a related GitHub issue.
  • I have added a description of the change (and Github issue number, if any) to the root CHANGELOG.md.

(Optional) Mobile checklist

Please ensure you completed the following checks if you did any changes to the mobile package:

  • I have run e2e tests for mobile
  • I have updated base screenshots for visual regression tests

@islathehut islathehut marked this pull request as ready for review May 13, 2024 19:21
Copy link

@leblowl leblowl left a 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 })
Copy link

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?

Copy link
Collaborator Author

@islathehut islathehut May 13, 2024

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.

@islathehut islathehut merged commit c317ffb into 2.2.0 May 13, 2024
22 of 23 checks passed
@islathehut islathehut deleted the fix-queue-processing branch May 13, 2024 22:22
leblowl pushed a commit that referenced this pull request May 14, 2024
* Process queue fixes and updates

* Remove unnecessary log

* Log when we push to queue
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