Fix/unify client errors - #850
Conversation
|
@Peolite001 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
…t-errors # Conflicts: # src/components/WalletContext.tsx
…01/ProofOfHeart-frontend into fix/unify-client-errors # Conflicts: # src/components/WalletContext.tsx
|
Auto-review failed (API error). Leaving PR for human review. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const parts = fullKey.split("."); | ||
| const key = parts[parts.length - 1]; | ||
| const namespace = parts.length > 1 ? parts[0] : ''; | ||
| const namespace = parts.length > 1 ? parts[0] : ""; |
| // Step 3: Check for withdrawal action button or navigate directly to withdraw tab | ||
| const withdrawBtn = page.getByRole("button", { name: /withdraw|claim/i }).or(page.locator("body")); | ||
| await expect(withdrawBtn).toBeVisible(); | ||
| const withdrawBtn = page.getByRole("button", { name: /withdraw|claim/i }).first(); |
|
Auto-review failed (API error). Leaving PR for human review. |
|
Auto-review failed (API error). Leaving PR for human review. |
davidmaronio
left a comment
davidmaronio
left a comment
There was a problem hiding this comment.
the unified ClientError with status/details is a sensible shape and the migration of both contractClient and offchainApiClient to throw it will simplify consumers. blocking issues:
- this branch is stacked on #849: its diff contains every file from that PR plus clientError.ts, contractClient.ts, and offchainApiClient.ts. as-is it cannot be reviewed or merged independently. please either mark it as depending on #849 and rebase once that lands, or rebase directly onto main so only the error-unification changes remain.
- branch is also in conflict with main, so a rebase is needed regardless.
happy to do a full pass on the contractClient/offchainApiClient changes once the diff is just this PR's work.
4 participants
Summary
This PR aligns the error-handling conventions across our on-chain (
contractClient.ts) and off-chain (offchainApiClient.ts) network layers by introducing a unifiedClientErrorclass.Previously,
offchainApiClient.tsthrew a specificOffchainApiErrorcontainingstatusanddetailsfields, whereascontractClient.tsthrew genericErrorinstances containing localized strings (and occasionally masking raw error objects). This made it difficult for components utilizing both clients to implement uniform error states or robust fallback logic without awkwardly branching based on error formats.By migrating both to the new
ClientError, consumers can now reliably expect errors shaped withstatusanddetailsalongside their parsed messages, regardless of whether a transaction was routed to the API or to Soroban.Closes #837
Type of Change
Contributor Checklist
CONTRIBUTING.mdfor branch, commit, and PR title conventions..rejects.toThrow()assertions incontractClient.test.tsstill evaluate accurately against the newClientError).Validation
npm run lintnpm run format:checknpm run typechecknpm testnpm run buildtscmissing from execution environment), but the changes are strictly structural refactors preserving existing behavior.Notes for Reviewers
src/lib/clientError.tswas introduced to house theClientErrorimplementation.OffchainApiErrorhas been entirely removed from the repository.