Skip to content

fix: throw when adding sub account owner fails#347

Open
Snowmanzx wants to merge 1 commit into
base:masterfrom
Snowmanzx:fix/sub-account-signer-throw
Open

fix: throw when adding sub account owner fails#347
Snowmanzx wants to merge 1 commit into
base:masterfrom
Snowmanzx:fix/sub-account-signer-throw

Conversation

@Snowmanzx

Copy link
Copy Markdown

sendRequestToSubAccountSigner swallowed errors when handleAddSubAccountOwner failed.

The catch block did return standardErrors.provider.unauthorized(...). That factory returns an EthereumProviderError, it does not throw. So the async function resolved with the error object as its value instead of rejecting. The only caller awaits the result and returns it so its catch never runs and the dApp gets the error object back as if it were a valid result.

Changed return to throw, which matches how the rest of the function reports errors (assertPresence throws the same error type above).

Added a test in the auto sub account block that makes handleAddSubAccountOwner reject and checks that request rejects.

Closes #326

The catch block in sendRequestToSubAccountSigner used return standardErrors.provider.unauthorized(...). That factory returns an error object so the promise resolved with the error instead of rejecting and the caller used it as a successful result. Changed return to throw. Added a regression test.
@cb-heimdall

Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sendRequestToSubAccountSigner silently returns Error object instead of throwing when handleAddSubAccountOwner fails

2 participants