Skip to content

sendRequestToSubAccountSigner silently returns Error object instead of throwing when handleAddSubAccountOwner fails #326

Description

@Nexory

File: packages/account-sdk/src/sign/base-account/Signer.ts

Affected lines: 789–798 (catch block inside sendRequestToSubAccountSigner)


What the code does

// 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):

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:

} 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).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions