base / base/account-sdk

sendRequestToSubAccountSigner silently returns Error object instead of throwing when handleAddSubAccountOwner fails

Open Beginner friendly
#326 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.