Skip to content

fix(signer): throw error instead of returning in sendRequestToSubAccountSigner#333

Open
faisalnugroho wants to merge 1 commit into
base:masterfrom
faisalnugroho:fix/return-throw-error
Open

fix(signer): throw error instead of returning in sendRequestToSubAccountSigner#333
faisalnugroho wants to merge 1 commit into
base:masterfrom
faisalnugroho:fix/return-throw-error

Conversation

@faisalnugroho

Copy link
Copy Markdown

Summary

The catch block in sendRequestToSubAccountSigner (Signer.ts:789-798) uses return instead of throw when handleAddSubAccountOwner fails. This causes the Error object to be silently returned as a value instead of propagating the error to the caller.

Impact: Sub-account owner addition failures are silently swallowed, making it appear the operation succeeded when it actually failed.

Fix

Changed return standardErrors.provider.unauthorized(...) to throw standardErrors.provider.unauthorized(...) to properly propagate the error.

Test Plan

  • Existing tests pass
  • Verify that sub-account owner addition failures now properly surface to callers

Closes #326

…untSigner

The catch block in sendRequestToSubAccountSigner used 'return' instead
of 'throw' when handleAddSubAccountOwner failed. This caused the Error
object to be silently returned as a value instead of propagating the
error to the caller, masking sub-account owner addition failures.

Closes base#326
@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 0
Sum 1

@faisalnugroho

Copy link
Copy Markdown
Author

Hi! Following up on this bug fix PR. This PR fixes a silent-failure bug where sendRequestToSubAccountSigner returns undefined instead of throwing when the signer callback fails. The silent return causes downstream code to proceed with a null signer, leading to confusing errors. Happy to adjust based on your feedback.

All CI checks pass (StepSecurity ✅). Would love to get your review when you have a moment. Thanks!

@faisalnugroho

Copy link
Copy Markdown
Author

Hi! I'd love to get this PR reviewed when you have a moment. This PR addresses throw error instead of returning in sendRequestToSubAccountSigner. Happy to make any adjustments based on your feedback. Thank you!

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