fix: can timeout when updating CFE version #4365
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #4365 +/- ##
======================================
- Coverage 71% 71% -0%
======================================
Files 394 394
Lines 67064 66828 -236
Branches 67064 66828 -236
======================================
- Hits 47925 47704 -221
+ Misses 16709 16692 -17
- Partials 2430 2432 +2 ☔ View full report in Codecov by Sentry. |
msgmaxim
force-pushed
the
fix/cfe-upgrade-gets-stuck
branch
from
deb51ac to
f1837cd
Compare
| .await? | ||
| .wait_for_in_block() | ||
| .await?; | ||
| tokio::time::timeout( |
There was a problem hiding this comment.
Just one thing can you add a comment, mentioning the issue? Of course in the end I agree we should add timeouts everywhere. But just while this is a one off timeout.
There was a problem hiding this comment.
Are you sure it is the second await? Because if the first await on line 778 is the problem, then this timeout will not work.
There was a problem hiding this comment.
You could put the whole request is an async block and put the timeout around that whole thing?
Pull Request
Closes: PRO-1064
Checklist
Please conduct a thorough self-review before opening the PR.
Summary
As discussed, this seems like the most appropriate solution at this time. I have tested this on localnet, both the happy path, and with the request timing out (by setting a very short timeout).