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

Ensure ready tasks are restarted as needed on resume #10119

Closed
wants to merge 1 commit into from

Conversation

mikeshardmind
Copy link
Contributor

Summary

A resume during startup chunking can result in erroring out while attempting to write to a closing transport.

fixes #10118

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@Rapptz
Copy link
Owner

Rapptz commented Feb 26, 2025

Hm.

I would have just checked if the task is incomplete within parse_resume and then restart it there. It feels easier to understand and doesn't hardcode the connection issue triggered. I suppose the immediate issue is that if it's a valid RESUME then it might be resuming the pre-existing chunking process, though I forget the specifics for this case in particular.

@mikeshardmind
Copy link
Contributor Author

I'm not sure if the ordering of resume dispatching vs this task erroring is reliable, or if it is currently, if it's easy to ensure it remains so, which is why I went with the done callback, as it will always be after the error has occurred if it is going to occur.

@mikeshardmind
Copy link
Contributor Author

There's something still not right about this ordering though based on user testing. the approach from the resume side is probably more correct.

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.

on_ready not firing due to RESUME while chunking upon bot start
2 participants