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

Jump to bottom

feat: surfacing error message returned from relayer #861

Merged
merged 5 commits into from
Sep 26, 2023
Merged

feat: surfacing error message returned from relayer #861

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ac82f5b
surfacing error message returned from relayer
Sep 14, 2023
0f59531
resolving PR feedback
Sep 26, 2023
4d5f264
fixed exclamation
Sep 26, 2023
c565665
Merge branch 'main' into feature/ID-1049-relay-error-message-surfacing
scastleman-immutable Sep 26, 2023
411887d
Merge branch 'main' into feature/ID-1049-relay-error-message-surfacing
scastleman-immutable Sep 26, 2023
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
5 changes: 3 additions & 2 deletions packages/passport/sdk/src/zkEvm/sendTransaction.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,10 @@ describe('sendTransaction', () => {
);
});

it('returns an error if the relayer does not return a successful status', async () => {
it('returns and surfaces an error if the relayer does not return a successful status', async () => {
(retryWithDelay as jest.Mock).mockResolvedValue({
status: RelayerTransactionStatus.FAILED,
statusMessage: 'Unable to complete transaction',
} as RelayerTransaction);

await expect(
Expand All @@ -152,7 +153,7 @@ describe('sendTransaction', () => {
).rejects.toThrow(
new JsonRpcError(
RpcErrorCode.INTERNAL_ERROR,
'Transaction failed to submit with status FAILED',
'Transaction failed to submit with status FAILED. Error message: Unable to complete transaction',
),
);
});
Expand Down
3 changes: 2 additions & 1 deletion packages/passport/sdk/src/zkEvm/sendTransaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ export const sendTransaction = ({
].includes(relayerTransaction.status)) {
throw new JsonRpcError(
RpcErrorCode.RPC_SERVER_ERROR,
`Transaction failed to submit with status ${relayerTransaction.status}`,
`Transaction failed to submit with status ${relayerTransaction.status}. `
+ `Error message: ${relayerTransaction.statusMessage}`,
Copy link
Contributor

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}`;
}

Copy link
Contributor Author

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.

);
}

Expand Down
1 change: 1 addition & 0 deletions packages/passport/sdk/src/zkEvm/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface RelayerTransaction {
chainId: string;
relayerId: string;
hash: string;
statusMessage: string;
}

export interface FeeOption {
Expand Down