-
Notifications
You must be signed in to change notification settings - Fork 39
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
improve deadline error #2015
improve deadline error #2015
Conversation
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.
lgtm
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.
LGTM
@@ -52,6 +54,7 @@ type ExecCfg struct { | |||
tryUntilAuthorised bool | |||
adjustArgs func(acct *GWAccount) []any | |||
cacheCfg *CacheCfg | |||
deadline time.Duration |
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.
Nitpick I'd probably call it timeout rather than deadline
@@ -34,13 +34,15 @@ const ( | |||
ethCallAddrPadding = "000000000000000000000000" | |||
|
|||
notAuthorised = "not authorised" | |||
serverBusy = "server busy. please retry later" |
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.
should it say retry? i.e. the transaction might have actually gone through, so you wouldn't want to send again, just go into reconcilliation
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.
the error response is generic for all rpc calls. The tx submission has a much higher timeout, so it shouldn't really be shown there, unless there is something really wrong
Why this change is needed