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.

Sign up for GitHub

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: error handling with retries when waiting for receipt #9650

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+8 −2
Open

fix: error handling with retries when waiting for receipt #9650

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
084abf3
fix: error handling with retries when waiting for receipt
grandizzy Jan 9, 2025
95fc7bb
Merge remote-tracking branch 'origin' into issue-9637
grandizzy Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions crates/script/src/receipts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub async fn check_tx_status(
return Ok(receipt.into());
}

let mut retries = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use the retry utility here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure we can since retries within Retry utility applies for all errors (while in this case we want to apply retry only for fatal errors while for timeout error we want to loop until receipt is available). does this make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this solution seems simple enough to me

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what the "until_break" variant is for. I would prefer using the Retry struct because it also emits a warning when retrying.

loop {
match PendingTransactionBuilder::new(provider.clone(), hash)
.with_timeout(Some(Duration::from_secs(timeout)))
Expand All @@ -48,8 +49,13 @@ pub async fn check_tx_status(
Ok(receipt) => return Ok(receipt.into()),
// do nothing on timeout, we will check whether tx is dropped below
Err(PendingTransactionError::TxWatcher(WatchTxError::Timeout)) => {}
// treat other errors as fatal
Err(e) => return Err(e.into()),
// treat other errors as fatal, with retries
Err(e) => {
if retries == 3 {
return Err(e.into())
}
retries += 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we log a debug here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what Retry is for.

}
}

if provider.get_transaction_by_hash(hash).await?.is_some() {
Expand Down
Loading