-
Notifications
You must be signed in to change notification settings - Fork 54
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
Handle panicking actors #1803
Handle panicking actors #1803
Conversation
task.run().await.map_err(|e| (running_name.clone(), e))?; | ||
|
||
Ok(running_name) | ||
match tokio::spawn(task.run()).await { |
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.
What was the benefit in spawning the task first and awaiting on it immediately over calling run()
directly? What was preventing the error mapping with the former direct run()
call version? From what I understood, returning the error directly with that ?
was the problem, right?
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.
returning the error directly with that ? was the problem, right?
No, this change doesn't concern the Result of run its about the run method panicking.
Originally, had the run method panicked the future would result in a Err(JoinError)
without me having the chance do anything in run_task, the JoinError doesn't know our actors name, which meant that the runtime on line 220 where it handles the JoinError, doesn't know which actor has finished and can't clean up the running actors or send the appropriate RuntimeEvent
.
By spawning another task here I can match on the result of that JoinHandle
and if its a Err(JoinError)
map it so I return the name of the actor.
00af8a5
to
757f703
Compare
Robot Results
Passed Tests
|
Proposed changes
Wrapped tasks.run() in a spawn so I can map the JoinErrors from that spawn into RuntimeErrors so that the runtime can do appropriate cleanup.
Types of changes
Paste Link to the issue
Checklist
cargo fmt
as mentioned in CODING_GUIDELINEScargo clippy
as mentioned in CODING_GUIDELINESFurther comments