-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: surfacing error message returned from relayer #861
feat: surfacing error message returned from relayer #861
Conversation
`Transaction failed to submit with status ${relayerTransaction.status}. ` | ||
+ `Error message: ${relayerTransaction.statusMessage}`, |
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.
I don't think the statusMessage
is always set 🤔 Can we update this to handle this scenario? And can I suggest we simplify the message a little bit here, e.g.:
let errorMessage = `Transaction failed to submit with status ${relayerTransaction.status}`;
if (relayerTransaction.statusMessage) {
errorMessage += `: ${relayerTransaction.statusMessage}`;
}
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.
Good call. I've added your logic there, and also changed the RelayerTransaction
interface so that statusMessage is optional.
@@ -123,9 +123,13 @@ export const sendTransaction = ({ | |||
RelayerTransactionStatus.SUBMITTED, | |||
RelayerTransactionStatus.SUCCESSFUL, | |||
].includes(relayerTransaction.status)) { | |||
let errorMessage = `Transaction failed to submit with status ${relayerTransaction.status}.`; | |||
if (!relayerTransaction.statusMessage) { |
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.
if (!relayerTransaction.statusMessage) { | |
if (relayerTransaction.statusMessage) { |
Summary
The
imGetTransactionByHash
method now returns an additionalstatusMessage
field on transaction failure, which contains information about the cause of the error. This PR captures that message and returns it to the end user.Why the changes
This helps convey the specific reason a request failed so that the end user can better respond to the probelm.
Things worth calling out
The error test in
sendTransaction.test.ts
has been updated to ensure this message is being returned, however I couldn't find a way to trigger an actual error in live testing, so there may be edge cases I was not able to capture.