sendRequestToSubAccountSigner silently returns Error object instead of throwing when handleAddSubAccountOwner fails
- Dominant language
- TypeScript
- Stars
- 172
- Forks
- 207
- Avg merge
- 49m
- Merged PRs (30d)
- 1
Description
**File:** `packages/account-sdk/src/sign/base-account/Signer.ts`
**Affected lines:** 789–798 (catch block inside `sendRequestToSubAccountSigner`)
---
### What the code does
```ts
// Signer.ts:782-798
try {
ownerIndex = await handleAddSubAccountOwner({ ... });
logAddOwnerCompleted({ ... });
} catch (error) {
logAddOwnerError({ ... });
return standardErrors.provider.unauthorized( // <-- BUG: return, not throw
'failed to add sub account owner when sending request to sub account signer'
);
}
```
`standardErrors.provider.unauthorized()` is a factory function (`errors.ts:56-58`) that constructs and **returns** a new `EthereumProviderError` instance. Using `return` here resolves the async function's Promise successfully with an `EthereumProviderError` object as the value, rather than rejecting the Promise.
---
### Why this is a bug
The sole call-site (`Signer.ts:184-199`):
```ts
try {
const result = await this.sendRequestToSubAccountSigner(request);
logSubAccountRequestCompleted({ ... });
return result as T; // receives Error object as successful result
} catch (error) {
logSubAccountRequestError({ ... });
throw error; // never reached for this failure mode
}
```
Because the Promise resolves (not rejects), the outer `catch` never fires. The `EthereumProviderError` object is returned up the stack and ultimately handed back to the dApp as if it were a valid RPC result. The error is silently swallowed.
---
### Expected behaviour
The catch block should `throw` the error so the Promise rejects and the caller's `catch` handles it:
```ts
} catch (error) {
logAddOwnerError({ ... });
throw standardErrors.provider.unauthorized(
'failed to add sub account owner when sending request to sub account signer'
);
}
```
---
### Impact
Any time `handleAddSubAccountOwner` throws (e.g. user rejects the add-owner popup, network error, contract revert), the failure is invisible to the dApp. The dApp receives an `EthereumProviderError` object as the resolved value of `eth_sendTransaction` / `wallet_sendCalls`, which it will likely try to use as a transaction hash or similar, producing a confusing downstream failure with no actionable error message.
---
### Reproduction path
1. Trigger a `wallet_sendCalls` or `eth_sendTransaction` request from a dApp using a sub-account whose owner index is `-1` (first-time use, or owner removed).
2. Make `handleAddSubAccountOwner` fail (e.g. reject the popup, or stub it to throw in a test).
3. Observe: the returned Promise resolves with an `EthereumProviderError` value instead of rejecting.
HEAD verified at commit `24ab30c` (account 2.5.6).
Contributor guide
Research direction
Start with packages/account-sdk/src/sign/base-account/Signer.ts:184–199 and 789–798, then inspect the standardErrors.provider.unauthorized factory in errors.ts:56–58. Reproduce the handleAddSubAccountOwner failure path and verify that sendRequestToSubAccountSigner rejects so the caller’s error handler runs instead of treating the error object as a successful result.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api
- Issue type
- Bug
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100