feat(*): improve connector errors #388
Conversation
WalkthroughThe changes refactor error handling throughout multiple packages by replacing ad hoc use of formatting functions (e.g. fmt.Errorf and errors.Wrap) with a dedicated utility function, errorsutils.NewWrappedError. Additionally, error unwrapping via errorsutils.Cause is introduced in several cases. The updates span API services, engine components, and numerous connector plugins, with refined error messages and adjusted types to avoid logging excessive stack traces. Changes
Sequence Diagram(s)(No sequence diagram generated as the changes primarily involve refactoring error handling without altering the overall control flow.) Assessment against linked issues
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
paul-nicolas
force-pushed
the
feat/improve-errors
branch
2 times, most recently
from
3dae1f6 to
77b52eb
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #388 +/- ##
==========================================
- Coverage 72.86% 72.35% -0.51%
==========================================
Files 531 533 +2
Lines 25884 26408 +524
==========================================
+ Hits 18860 19108 +248
- Misses 6006 6279 +273
- Partials 1018 1021 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
internal/connectors/plugins/public/stripe/reverse_transfers.go (1)
26-26: Fix typo in error messageThere's a typo in the error message: "if" should be "is".
- fmt.Errorf("transfer id if required in metadata of transfer reversal request"), + fmt.Errorf("transfer id is required in metadata of transfer reversal request"),internal/connectors/plugins/public/currencycloud/client/contacts.go (1)
43-47:❓ Verification inconclusive
Redundant error wrapping in HTTP request failure.
The error handling here creates redundancy because you're including
errRes.Error()in the message and also wrapping the originalerr. This could lead to duplicated error information in logs.Consider simplifying to:
- return nil, errorsutils.NewWrappedError( - fmt.Errorf("failed to get contacts: %v", errRes.Error()), - err, - ) + return nil, errorsutils.NewWrappedError( + fmt.Errorf("failed to get contacts: %v", errRes.Error()), + models.ErrInvalidRequest, + )Or alternatively:
- return nil, errorsutils.NewWrappedError( - fmt.Errorf("failed to get contacts: %v", errRes.Error()), - err, - ) + return nil, errorsutils.NewWrappedError( + fmt.Errorf("failed to get contacts"), + err, + )
🏁 Script executed:
#!/bin/bash # Check test coverage for these error handling changes rg -A 3 "TestGetContactID" --type goLength of output: 36
Attention: Manual Verification Required for Simplified Error Handling
While the proposed change to simplify error wrapping generally improves log clarity and avoids redundant error messages, our automated search (using “rg -A 3 TestGetContactID”) did not yield any test references. This lack of output suggests that either the tests for this code path are missing or they don’t explicitly cover this error condition. Please verify manually that the modified error handling does not negatively affect any error-dependent behaviors or logging expectations.
- Location:
internal/connectors/plugins/public/currencycloud/client/contacts.go(lines 43–47)- Issue: Redundant error wrapping may duplicate error details in logs.
- Suggested Changes:
Option 1:Option 2:- return nil, errorsutils.NewWrappedError( - fmt.Errorf("failed to get contacts: %v", errRes.Error()), - err, - ) + return nil, errorsutils.NewWrappedError( + fmt.Errorf("failed to get contacts: %v", errRes.Error()), + models.ErrInvalidRequest, + )- return nil, errorsutils.NewWrappedError( - fmt.Errorf("failed to get contacts: %v", errRes.Error()), - err, - ) + return nil, errorsutils.NewWrappedError( + fmt.Errorf("failed to get contacts"), + err, + )Please review and manually verify that these changes integrate well with the tests and maintain the intended error handling behavior.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 43-46: internal/connectors/plugins/public/currencycloud/client/contacts.go#L43-L46
Added lines #L43 - L46 were not covered by testsinternal/connectors/plugins/public/modulr/client/error.go (1)
19-34: Method name could be improved to better reflect its purposeThe method
Error()returns anerrortype instead of a string, which is different from the standard Goerrorinterface implementation. This might be confusing to other developers.Consider renaming this method to something like
ToError()orAsError()to better indicate that it's converting the collection to a single error:-func (mes modulrErrors) Error() error { +func (mes modulrErrors) ToError() error {Additionally, consider enhancing the error message to include information about additional errors if the slice contains more than one error:
me := mes[0] + + var errMsg string + if len(mes) > 1 { + errMsg = fmt.Sprintf("%d: %s (and %d more errors)", me.StatusCode, me.Message, len(mes)-1) + } else if me.Message == "" { + errMsg = fmt.Sprintf("unexpected status code: %d", me.StatusCode) + } else { + errMsg = fmt.Sprintf("%d: %s", me.StatusCode, me.Message) + } + + return errors.New(errMsg)internal/connectors/plugins/public/adyen/client/client.go (1)
84-86: Consider standardizing server error handling.For consistency with the client error handling improvements, consider using
errorsutils.NewWrappedErrorfor server errors as well.- if statusCode >= http.StatusInternalServerError { - return fmt.Errorf("unexpected status code %d: %w", statusCode, err) - } + if statusCode >= http.StatusInternalServerError { + return errorsutils.NewWrappedError( + fmt.Errorf("unexpected status code %d", statusCode), + err, + ) + }internal/connectors/plugins/public/bankingcircle/client/auth.go (1)
45-48: Consistent error wrapping pattern.The error handling follows the same structured pattern used throughout the PR, which creates consistency and improves error clarity by separating contextual information from the original error.
Consider adding tests to cover these error handling paths, as static analysis shows these lines aren't covered by tests.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 45-48: internal/connectors/plugins/public/bankingcircle/client/auth.go#L45-L48
Added lines #L45 - L48 were not covered by testsinternal/connectors/engine/engine.go (1)
325-325: Maintaining consistent error handling pattern.Another instance of the improved error wrapping approach using
errorsutils.NewWrappedError(). Line 310 still uses the old approach witherrors.Wrap- consider updating that as well for consistency.- return errors.Wrap(ErrValidation, err.Error()) + return errorsutils.NewWrappedError(err, ErrValidation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
internal/api/services/errors.go(2 hunks)internal/api/v2/errors.go(2 hunks)internal/connectors/engine/activities/errors.go(4 hunks)internal/connectors/engine/engine.go(6 hunks)internal/connectors/engine/errors.go(1 hunks)internal/connectors/engine/workflow/create_payout.go(3 hunks)internal/connectors/engine/workflow/create_transfer.go(3 hunks)internal/connectors/engine/workflow/update_tasks.go(2 hunks)internal/connectors/plugins/currency/currency.go(2 hunks)internal/connectors/plugins/public/adyen/client/client.go(2 hunks)internal/connectors/plugins/public/atlar/bank_account_creation.go(2 hunks)internal/connectors/plugins/public/atlar/bank_account_creation_test.go(3 hunks)internal/connectors/plugins/public/atlar/client/client.go(2 hunks)internal/connectors/plugins/public/atlar/client/counter_parties.go(2 hunks)internal/connectors/plugins/public/atlar/payouts.go(2 hunks)internal/connectors/plugins/public/atlar/payouts_test.go(3 hunks)internal/connectors/plugins/public/atlar/utils.go(2 hunks)internal/connectors/plugins/public/bankingcircle/client/accounts.go(3 hunks)internal/connectors/plugins/public/bankingcircle/client/auth.go(2 hunks)internal/connectors/plugins/public/bankingcircle/client/client.go(2 hunks)internal/connectors/plugins/public/bankingcircle/client/payments.go(4 hunks)internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go(2 hunks)internal/connectors/plugins/public/bankingcircle/payouts.go(2 hunks)internal/connectors/plugins/public/bankingcircle/payouts_test.go(7 hunks)internal/connectors/plugins/public/bankingcircle/transfers.go(2 hunks)internal/connectors/plugins/public/bankingcircle/transfers_test.go(7 hunks)internal/connectors/plugins/public/currencycloud/client/accounts.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/auth.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/balances.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/beneficiaries.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/contacts.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/payouts.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/transactions.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/transfers.go(2 hunks)internal/connectors/plugins/public/currencycloud/payouts.go(2 hunks)internal/connectors/plugins/public/currencycloud/payouts_test.go(3 hunks)internal/connectors/plugins/public/currencycloud/transfers.go(2 hunks)internal/connectors/plugins/public/currencycloud/transfers_test.go(3 hunks)internal/connectors/plugins/public/generic/balances.go(1 hunks)internal/connectors/plugins/public/mangopay/bank_account_creation.go(6 hunks)internal/connectors/plugins/public/mangopay/bank_account_creation_test.go(5 hunks)internal/connectors/plugins/public/mangopay/client/bank_accounts.go(3 hunks)internal/connectors/plugins/public/mangopay/client/payin.go(2 hunks)internal/connectors/plugins/public/mangopay/client/payout.go(3 hunks)internal/connectors/plugins/public/mangopay/client/refund.go(2 hunks)internal/connectors/plugins/public/mangopay/client/transactions.go(2 hunks)internal/connectors/plugins/public/mangopay/client/transfer.go(3 hunks)internal/connectors/plugins/public/mangopay/client/users.go(2 hunks)internal/connectors/plugins/public/mangopay/client/wallets.go(3 hunks)internal/connectors/plugins/public/mangopay/client/webhooks.go(4 hunks)internal/connectors/plugins/public/mangopay/payouts.go(3 hunks)internal/connectors/plugins/public/mangopay/payouts_test.go(6 hunks)internal/connectors/plugins/public/mangopay/plugin.go(3 hunks)internal/connectors/plugins/public/mangopay/transfers.go(2 hunks)internal/connectors/plugins/public/mangopay/transfers_test.go(5 hunks)internal/connectors/plugins/public/mangopay/webhooks.go(2 hunks)internal/connectors/plugins/public/mangopay/webhooks_test.go(3 hunks)internal/connectors/plugins/public/modulr/client/accounts.go(3 hunks)internal/connectors/plugins/public/modulr/client/beneficiaries.go(2 hunks)internal/connectors/plugins/public/modulr/client/error.go(2 hunks)internal/connectors/plugins/public/modulr/client/payments.go(2 hunks)internal/connectors/plugins/public/modulr/client/payout.go(3 hunks)internal/connectors/plugins/public/modulr/client/transactions.go(2 hunks)internal/connectors/plugins/public/modulr/client/transfer.go(3 hunks)internal/connectors/plugins/public/modulr/payouts.go(2 hunks)internal/connectors/plugins/public/modulr/payouts_test.go(3 hunks)internal/connectors/plugins/public/modulr/transfers.go(2 hunks)internal/connectors/plugins/public/modulr/transfers_test.go(3 hunks)internal/connectors/plugins/public/modulr/utils.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/accounts.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/auth.go(1 hunks)internal/connectors/plugins/public/moneycorp/client/balances.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/error.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/payouts.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/recipients.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/transactions.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/transfers.go(3 hunks)internal/connectors/plugins/public/moneycorp/payouts.go(2 hunks)internal/connectors/plugins/public/moneycorp/payouts_test.go(3 hunks)internal/connectors/plugins/public/moneycorp/transfers.go(2 hunks)internal/connectors/plugins/public/moneycorp/transfers_test.go(3 hunks)internal/connectors/plugins/public/moneycorp/utils.go(1 hunks)internal/connectors/plugins/public/stripe/client/client.go(2 hunks)internal/connectors/plugins/public/stripe/create_payouts.go(2 hunks)internal/connectors/plugins/public/stripe/create_payouts_test.go(2 hunks)internal/connectors/plugins/public/stripe/create_transfers.go(2 hunks)internal/connectors/plugins/public/stripe/create_transfers_test.go(2 hunks)internal/connectors/plugins/public/stripe/reverse_transfers.go(2 hunks)internal/connectors/plugins/public/stripe/reverse_transfers_test.go(1 hunks)internal/connectors/plugins/public/stripe/utils.go(1 hunks)internal/connectors/plugins/public/wise/client/balances.go(3 hunks)internal/connectors/plugins/public/wise/client/payouts.go(3 hunks)internal/connectors/plugins/public/wise/client/profiles.go(2 hunks)internal/connectors/plugins/public/wise/client/quotes.go(2 hunks)internal/connectors/plugins/public/wise/client/recipient_accounts.go(3 hunks)internal/connectors/plugins/public/wise/client/transfers.go(4 hunks)internal/connectors/plugins/public/wise/client/webhooks.go(4 hunks)internal/connectors/plugins/public/wise/config.go(2 hunks)internal/connectors/plugins/public/wise/payouts.go(3 hunks)internal/connectors/plugins/public/wise/payouts_test.go(7 hunks)internal/connectors/plugins/public/wise/transfers.go(2 hunks)internal/connectors/plugins/public/wise/transfers_test.go(7 hunks)internal/connectors/plugins/public/wise/utils.go(1 hunks)internal/connectors/plugins/registry/errors.go(2 hunks)internal/connectors/plugins/registry/plugins.go(1 hunks)internal/utils/errors/errors.go(1 hunks)internal/utils/errors/errors_test.go(1 hunks)
⛔ Files not processed due to max files limit (1)
- test/e2e/api_connectors_test.go
🧰 Additional context used
🧬 Code Definitions (18)
internal/connectors/plugins/registry/errors.go (2)
internal/connectors/plugins/errors.go (2)
ErrInvalidClientRequest(10-10)ErrUpstreamRatelimit(11-11)internal/connectors/httpwrapper/client.go (1)
ErrStatusCodeTooManyRequests(21-21)
internal/connectors/plugins/public/mangopay/bank_account_creation.go (1)
internal/models/errors.go (1)
ErrFailedAccountCreation(10-10)
internal/api/v2/errors.go (2)
internal/api/v3/errors.go (1)
ErrValidation(14-14)internal/storage/error.go (1)
ErrValidation(16-16)
internal/connectors/plugins/public/stripe/client/client.go (1)
internal/connectors/httpwrapper/client.go (2)
ErrStatusCodeTooManyRequests(21-21)ErrStatusCodeClientError(19-19)
internal/connectors/engine/activities/errors.go (3)
internal/connectors/plugins/errors.go (3)
ErrNotImplemented(8-8)ErrInvalidClientRequest(10-10)ErrCurrencyNotSupported(12-12)internal/storage/error.go (1)
ErrNotFound(17-17)internal/models/errors.go (1)
ErrMissingConnectorMetadata(15-15)
internal/connectors/plugins/public/mangopay/plugin.go (3)
internal/models/plugin.go (1)
TranslateWebhookResponse(142-144)internal/models/errors.go (1)
ErrInvalidRequest(13-13)internal/connectors/plugins/public/mangopay/client/webhooks.go (2)
Webhook(71-75)EventType(14-14)
internal/connectors/plugins/registry/plugins.go (15)
internal/models/config.go (1)
Config(14-18)internal/connectors/plugins/registry/plugins_test.go (1)
Config(22-33)internal/connectors/httpwrapper/config.go (1)
Config(10-16)internal/connectors/plugins/public/wise/config.go (1)
Config(16-21)internal/connectors/plugins/public/atlar/config.go (1)
Config(11-15)internal/connectors/plugins/public/adyen/config.go (1)
Config(11-19)internal/connectors/plugins/public/bankingcircle/config.go (1)
Config(11-18)internal/connectors/plugins/public/currencycloud/config.go (1)
Config(11-15)internal/connectors/plugins/public/dummypay/config.go (1)
Config(10-12)internal/connectors/plugins/public/generic/config.go (1)
Config(11-14)internal/connectors/plugins/public/mangopay/config.go (1)
Config(11-15)internal/connectors/plugins/public/modulr/config.go (1)
Config(11-15)internal/connectors/plugins/public/moneycorp/config.go (1)
Config(11-15)internal/connectors/plugins/public/stripe/config.go (1)
Config(11-13)internal/connectors/plugins/registry/configs.go (1)
Config(14-14)
internal/connectors/plugins/public/mangopay/payouts.go (1)
internal/models/errors.go (1)
ErrInvalidRequest(13-13)
internal/connectors/plugins/public/mangopay/webhooks.go (1)
internal/models/errors.go (1)
ErrInvalidRequest(13-13)
internal/connectors/plugins/public/atlar/bank_account_creation.go (1)
internal/models/plugin.go (1)
CreateBankAccountResponse(116-118)
internal/connectors/engine/engine.go (5)
internal/connectors/engine/plugins/plugin.go (1)
ErrValidation(23-23)internal/connectors/engine/errors.go (1)
ErrValidation(11-11)internal/api/services/errors.go (1)
ErrValidation(12-12)internal/api/v2/errors.go (1)
ErrValidation(19-19)internal/models/tasks.go (1)
Task(19-33)
internal/connectors/plugins/public/atlar/client/client.go (1)
internal/connectors/httpwrapper/client.go (3)
ErrStatusCodeTooManyRequests(21-21)ErrStatusCodeClientError(19-19)ErrStatusCodeServerError(20-20)
internal/connectors/plugins/public/moneycorp/utils.go (1)
internal/connectors/plugins/public/moneycorp/plugin.go (1)
Plugin(22-27)
internal/connectors/plugins/public/stripe/utils.go (1)
internal/connectors/plugins/public/stripe/plugin.go (1)
Plugin(22-26)
internal/connectors/plugins/public/modulr/client/transfer.go (3)
internal/connectors/plugins/public/currencycloud/client/transfers.go (1)
TransferResponse(41-56)internal/connectors/plugins/public/mangopay/client/transfer.go (1)
TransferResponse(29-45)internal/connectors/plugins/public/moneycorp/client/transfers.go (1)
TransferResponse(50-53)
internal/connectors/plugins/public/adyen/client/client.go (1)
internal/connectors/httpwrapper/client.go (2)
ErrStatusCodeTooManyRequests(21-21)ErrStatusCodeClientError(19-19)
internal/connectors/plugins/public/bankingcircle/payouts.go (2)
internal/models/bank_accounts.go (2)
BankAccountAccountNumberMetadataKey(19-19)BankAccountIBANMetadataKey(20-20)internal/connectors/plugins/public/bankingcircle/client/accounts.go (1)
Account(29-40)
internal/connectors/plugins/public/currencycloud/payouts.go (1)
internal/connectors/plugins/public/currencycloud/plugin.go (1)
Plugin(22-27)
🪛 GitHub Check: codecov/patch
internal/connectors/plugins/public/atlar/client/counter_parties.go
[warning] 70-73: internal/connectors/plugins/public/atlar/client/counter_parties.go#L70-L73
Added lines #L70 - L73 were not covered by tests
internal/connectors/plugins/public/currencycloud/client/balances.go
[warning] 57-60: internal/connectors/plugins/public/currencycloud/client/balances.go#L57-L60
Added lines #L57 - L60 were not covered by tests
internal/connectors/plugins/public/currencycloud/client/accounts.go
[warning] 53-56: internal/connectors/plugins/public/currencycloud/client/accounts.go#L53-L56
Added lines #L53 - L56 were not covered by tests
internal/connectors/plugins/public/currencycloud/client/contacts.go
[warning] 43-46: internal/connectors/plugins/public/currencycloud/client/contacts.go#L43-L46
Added lines #L43 - L46 were not covered by tests
[warning] 50-53: internal/connectors/plugins/public/currencycloud/client/contacts.go#L50-L53
Added lines #L50 - L53 were not covered by tests
internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go
[warning] 60-63: internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go#L60-L63
Added lines #L60 - L63 were not covered by tests
internal/connectors/plugins/public/currencycloud/client/beneficiaries.go
[warning] 56-59: internal/connectors/plugins/public/currencycloud/client/beneficiaries.go#L56-L59
Added lines #L56 - L59 were not covered by tests
internal/connectors/engine/activities/errors.go
[warning] 45-45: internal/connectors/engine/activities/errors.go#L45
Added line #L45 was not covered by tests
[warning] 61-61: internal/connectors/engine/activities/errors.go#L61
Added line #L61 was not covered by tests
[warning] 73-73: internal/connectors/engine/activities/errors.go#L73
Added line #L73 was not covered by tests
internal/connectors/plugins/public/currencycloud/client/auth.go
[warning] 40-43: internal/connectors/plugins/public/currencycloud/client/auth.go#L40-L43
Added lines #L40 - L43 were not covered by tests
internal/connectors/plugins/public/bankingcircle/transfers.go
[warning] 47-50: internal/connectors/plugins/public/bankingcircle/transfers.go#L47-L50
Added lines #L47 - L50 were not covered by tests
internal/connectors/engine/errors.go
[warning] 15-25: internal/connectors/engine/errors.go#L15-L25
Added lines #L15 - L25 were not covered by tests
[warning] 29-29: internal/connectors/engine/errors.go#L29
Added line #L29 was not covered by tests
internal/connectors/plugins/public/atlar/bank_account_creation.go
[warning] 50-54: internal/connectors/plugins/public/atlar/bank_account_creation.go#L50-L54
Added lines #L50 - L54 were not covered by tests
internal/connectors/engine/engine.go
[warning] 567-567: internal/connectors/engine/engine.go#L567
Added line #L567 was not covered by tests
[warning] 685-685: internal/connectors/engine/engine.go#L685
Added line #L685 was not covered by tests
internal/connectors/plugins/public/atlar/client/client.go
[warning] 104-104: internal/connectors/plugins/public/atlar/client/client.go#L104
Added line #L104 was not covered by tests
[warning] 108-108: internal/connectors/plugins/public/atlar/client/client.go#L108
Added line #L108 was not covered by tests
[warning] 110-110: internal/connectors/plugins/public/atlar/client/client.go#L110
Added line #L110 was not covered by tests
internal/connectors/plugins/public/bankingcircle/client/auth.go
[warning] 40-43: internal/connectors/plugins/public/bankingcircle/client/auth.go#L40-L43
Added lines #L40 - L43 were not covered by tests
[warning] 45-48: internal/connectors/plugins/public/bankingcircle/client/auth.go#L45-L48
Added lines #L45 - L48 were not covered by tests
internal/connectors/plugins/public/bankingcircle/client/accounts.go
[warning] 75-78: internal/connectors/plugins/public/bankingcircle/client/accounts.go#L75-L78
Added lines #L75 - L78 were not covered by tests
[warning] 98-101: internal/connectors/plugins/public/bankingcircle/client/accounts.go#L98-L101
Added lines #L98 - L101 were not covered by tests
internal/connectors/plugins/public/adyen/client/client.go
[warning] 75-75: internal/connectors/plugins/public/adyen/client/client.go#L75
Added line #L75 was not covered by tests
[warning] 79-79: internal/connectors/plugins/public/adyen/client/client.go#L79
Added line #L79 was not covered by tests
internal/connectors/plugins/public/bankingcircle/client/payments.go
[warning] 123-126: internal/connectors/plugins/public/bankingcircle/client/payments.go#L123-L126
Added lines #L123 - L126 were not covered by tests
[warning] 147-150: internal/connectors/plugins/public/bankingcircle/client/payments.go#L147-L150
Added lines #L147 - L150 were not covered by tests
[warning] 174-177: internal/connectors/plugins/public/bankingcircle/client/payments.go#L174-L177
Added lines #L174 - L177 were not covered by tests
internal/connectors/plugins/public/bankingcircle/payouts.go
[warning] 62-65: internal/connectors/plugins/public/bankingcircle/payouts.go#L62-L65
Added lines #L62 - L65 were not covered by tests
🔇 Additional comments (308)
internal/connectors/plugins/public/stripe/create_payouts_test.go (2)
77-77: Improved error message specificity for better debuggingThe error message now clearly indicates that the destination account is specifically required in the context of a transfer/payout request, which provides better guidance to API users.
90-90: Enhanced error details with specific asset identifierThe error message now includes the problematic asset identifier "HHH", making it easier to identify and fix the issue. This type of contextual information in error messages helps significantly with troubleshooting.
internal/connectors/plugins/public/stripe/reverse_transfers.go (1)
25-28: The error wrapping approach is consistent with PR objectivesThe implementation correctly uses the
errorsutils.NewWrappedErrorfunction to wrap the validation error, which aligns with the PR objective of improving connector error handling.internal/connectors/plugins/public/currencycloud/client/balances.go (2)
57-60: Consistent use of error wrapping patternThe implementation correctly uses
errorsutils.NewWrappedErrorto provide a clear structure for wrapped errors, preserving the original error context.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 57-60: internal/connectors/plugins/public/currencycloud/client/balances.go#L57-L60
Added lines #L57 - L60 were not covered by tests
57-60:❓ Verification inconclusive
Consider adding test coverage for error handling
The error handling code you've added is not covered by tests according to static analysis. Consider adding test cases to ensure this error path works as expected.
🏁 Script executed:
#!/bin/bash # Check if there are any tests for this file echo "Checking for test files related to currencycloud client balances..." fd -t f balances_test.go internal/connectors/plugins/public/currencycloud/client # Check if there are any tests that might cover the error handling echo "Checking for potential tests of the GetBalances function..." rg -l "TestGetBalances|test.*GetBalances" internal/connectors/plugins/public/currencycloud/client # Look for mock definitions that might be used in tests echo "Checking for mock definitions..." rg -l "Mock.*httpClient|Mock.*client" internal/connectors/plugins/public/currencycloud/clientLength of output: 627
Attention: Confirm Test Coverage for Error Handling in CurrencyCloud Client
It appears that the error branch in
internal/connectors/plugins/public/currencycloud/client/balances.go(lines 57–60) is not clearly exercised by existing tests. Our automated search did not return any test files or mocks covering theGetBalanceserror scenario—though the output was minimal, so the evidence is not fully conclusive. Please verify manually whether this error handling path is indeed untested, and if so, add appropriate tests (including mocks for HTTP client failures) to ensure robust coverage.
- Location:
internal/connectors/plugins/public/currencycloud/client/balances.go(lines 57–60)🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 57-60: internal/connectors/plugins/public/currencycloud/client/balances.go#L57-L60
Added lines #L57 - L60 were not covered by testsinternal/connectors/plugins/public/mangopay/webhooks.go (2)
14-14: Import of error utilities package addedGood addition of the error utilities package which will be used for improved error handling.
105-108: Improved error handling with better messageThe error handling has been improved in two ways:
- Now using
errorsutils.NewWrappedErrorinstead of direct formatting- Error message is more user-focused ("webhook base URL is required" vs likely mentioning an environment variable)
This change aligns with the PR objective to enhance error handling and improve error messages for connectors.
internal/connectors/plugins/public/currencycloud/client/payouts.go (2)
13-13: Import of error utilities package addedGood addition of the error utilities package which will be used for improved error handling.
78-82: Enhanced error handling with structured approachGood change to use
errorsutils.NewWrappedErrorinstead of direct error formatting. This provides a more structured way to wrap the original error while adding context about the failure reason.This implementation will help the PR achieve its goal of improving error messages for connectors and providing more meaningful error responses to users.
internal/connectors/plugins/public/currencycloud/client/transactions.go (2)
11-11: Import of error utilities package addedGood addition of the error utilities package which will be used for improved error handling.
71-75: Structured error handling implementationGood refactoring to use
errorsutils.NewWrappedErrorwhich provides a more consistent approach to error handling across the codebase. This change:
- Maintains the original error as context
- Provides a clear message about what failed
- Aligns with the PR's goal of improving connector error messages
The implementation correctly formats the error message and wraps the original error.
internal/connectors/plugins/public/generic/balances.go (2)
11-11: Import of error utilities package addedGood addition of the error utilities package which will be used for improved error handling.
17-20: Standardized error handling approachGood implementation of the standardized error handling approach using
errorsutils.NewWrappedError. This change:
- Maintains the semantics of the original error (wrapping with
models.ErrInvalidRequest)- Provides a clear error message
- Follows the consistent pattern being applied across the codebase
This change supports the PR objective to improve error handling and provide better error messages when a request fails due to user input.
internal/connectors/plugins/registry/errors.go (2)
22-25: Improved error wrapping for client-side errorsThe switch to
errorsutils.NewWrappedErrorstandardizes error handling for client-side errors, maintaining the original error context while correctly categorizing it as anErrInvalidClientRequest. This aligns well with the PR objective of sendingBadRequestresponses instead of internal server errors for user input failures.
27-30: Standardized error handling for rate limiting errorsUsing
errorsutils.NewWrappedErrorfor rate limiting errors provides consistent error handling throughout the codebase. This approach properly preserves the original error context while categorizing it appropriately as an upstream rate limit issue.internal/connectors/plugins/public/currencycloud/client/transfers.go (1)
77-80: Enhanced error handling for transfer creation failuresThe refactoring improves error reporting by using
errorsutils.NewWrappedErrorto wrap the error response with the original error. This ensures that error context is preserved while providing a clearer error message structure.internal/connectors/plugins/public/wise/client/quotes.go (1)
47-50: Improved error handling and return valuesThe implementation now:
- Uses
errorsutils.NewWrappedErrorfor consistent error handling- Returns an empty
Quote{}struct instead of the partialquotevariable in the error case- Preserves the original error context for better debugging
These changes align with the PR objective of improving error handling for connector operations.
internal/connectors/plugins/public/currencycloud/client/auth.go (1)
40-43:❓ Verification inconclusive
Standardized authentication error handling
The implementation now wraps authentication errors using
errorsutils.NewWrappedError, consistent with the approach used in other files. This improves error context preservation and standardizes error handling throughout the codebase.
🏁 Script executed:
#!/bin/bash # Check if the new error handling code is covered by tests # Look for tests that exercise the authenticate method rg -A 5 -B 5 "authenticate.*error" --type go --glob "*test.go" # Check if there are any specific tests for authentication error scenarios rg -A 5 -B 5 "TestAuthenticate.*Error" --type go --glob "*test.go"Length of output: 133
Verify Test Coverage for Authentication Error Handling
The changes now encapsulate authentication errors using
errorsutils.NewWrappedError, aligning with our standardized approach. However, our initial automated search didn’t reveal test cases that explicitly exercise these error paths in the authentication logic. Please confirm that there are tests validating this behavior—or add them if missing—to ensure robust regression protection.
- File to review:
internal/connectors/plugins/public/currencycloud/client/auth.go(lines 40-43)- Action: Manually verify (or add) tests that simulate authentication failures and confirm that errors are properly wrapped.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 40-43: internal/connectors/plugins/public/currencycloud/client/auth.go#L40-L43
Added lines #L40 - L43 were not covered by testsinternal/connectors/plugins/public/stripe/utils.go (1)
7-7: Improved error handling structureThe change to import
errorsutilsand useNewWrappedErrorinstead of basic error formatting is a positive improvement. This approach provides better structure to errors by clearly separating the specific error message from the error type (models.ErrInvalidRequest), which will help upstream code determine the appropriate HTTP response code (likely a 400 Bad Request instead of 500 Internal Server Error).Also applies to: 12-15
internal/connectors/plugins/public/moneycorp/utils.go (1)
7-7: Consistent error handling approachThe changes follow the same pattern as in other connectors, moving from basic error formatting to using
errorsutils.NewWrappedError. This maintains consistent error handling across all connector plugins and ensures validation errors are properly categorized asmodels.ErrInvalidRequest. This will help the API layer respond with appropriate HTTP status codes when validations fail.Also applies to: 12-15, 19-22
internal/connectors/plugins/public/wise/client/profiles.go (1)
9-9: Improved error handling and return semanticsTwo important improvements in this change:
- Using
errorsutils.NewWrappedErrorfor better structured error handling- Returning
nilinstead of an empty slice when an error occursReturning
nilis more idiomatic in Go as it clearly indicates operation failure, while an empty slice could be confused with a successful operation that simply found no profiles.Also applies to: 29-32
internal/connectors/plugins/public/modulr/payouts.go (2)
12-12: Enhanced error handling with improved contextThe change to use
errorsutils.NewWrappedErroraligns with the broader error handling improvements. The error message now clearly indicates what operation failed ("failed to get currency and precision from asset"), which will make debugging easier.Also applies to: 22-25
30-33: Improved error details for debuggingThe error message now includes the problematic amount value (
pi.Amount) in the error message. This is a good practice as it immediately provides the value that caused the failure, making it easier to diagnose issues without having to reproduce them.internal/connectors/plugins/public/stripe/create_transfers.go (2)
14-14: Good addition of error utilities.Adding the errorsutils package enables more structured error handling throughout the connector code.
25-28: Improved error handling for validation errors.The change from direct error formatting to using
errorsutils.NewWrappedErrorproperly associates validation errors withmodels.ErrInvalidRequest. This ensures that currency/precision issues will now result in a BadRequest response rather than an internal server error, providing better feedback to API consumers.internal/connectors/plugins/public/atlar/utils.go (3)
10-10: Good addition of error utilities.Adding the errorsutils package enables more structured error handling throughout the connector code.
19-22: Improved error handling for source account validation.The change from direct error formatting to using
errorsutils.NewWrappedErrorproperly associates validation errors withmodels.ErrInvalidRequest. This ensures that missing source account errors will now result in a BadRequest response rather than an internal server error.
26-29: Improved error handling for destination account validation.The change from direct error formatting to using
errorsutils.NewWrappedErrorproperly associates validation errors withmodels.ErrInvalidRequest. This ensures that missing destination account errors will now result in a BadRequest response rather than an internal server error.internal/connectors/plugins/public/currencycloud/client/beneficiaries.go (2)
10-10: Good addition of error utilities.Adding the errorsutils package enables more structured error handling throughout the connector code.
56-59:❓ Verification inconclusive
Improved error handling with detailed wrapping.
Using
errorsutils.NewWrappedErrorprovides both the CurrencyCloud-specific error message and preserves the underlying error context, enhancing error reporting and debugging.The static analysis indicates this code isn't covered by tests. Consider adding test coverage:
🏁 Script executed:
#!/bin/bash # Check the test coverage for the beneficiaries.go file rg -l "Test.*GetBeneficiaries" internal/connectors/plugins/public/currencycloud/Length of output: 82
Error wrapping is improved, but test coverage for GetBeneficiaries remains unverified.
The updated error handling using
errorsutils.NewWrappedErrorclearly enhances context propagation by encapsulating both the CurrencyCloud-specific message and the underlying error. However, our initial static analysis (and the lack of output from the test search command) suggests that there are no tests covering this functionality. Please verify manually whether tests exist for theGetBeneficiariescode path, and if not, consider adding comprehensive tests to cover these error conditions.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 56-59: internal/connectors/plugins/public/currencycloud/client/beneficiaries.go#L56-L59
Added lines #L56 - L59 were not covered by testsinternal/connectors/engine/workflow/update_tasks.go (2)
6-6: Good addition of error utilities.Adding the errorsutils package enables more structured error handling throughout the workflow code.
16-16: Improved error storage by extracting the root cause.Using
errorsutils.Causeto extract just the root cause error for storage is an excellent improvement. This prevents storing entire error chains or stack traces in the database, aligning perfectly with the PR objective to avoid storing excessive error information. This will lead to:
- More concise error storage in the database
- Better readability of error logs
- Reduced database storage requirements
Also applies to: 24-24
internal/connectors/plugins/public/atlar/payouts_test.go (3)
80-80: Improved error message specificityThe error message now clearly indicates that the source account is required specifically "in transfer/payout request", making it more descriptive and easier to understand the context of the validation failure.
93-93: Improved error message specificitySimilarly to the source account error, this error message now clearly indicates that the destination account is required specifically "in transfer/payout request", providing better context for the validation error.
106-106: Enhanced error message with specific asset informationThe error message now includes the specific asset "HUF" that failed validation, replacing the generic "missing currencies" message. This makes troubleshooting easier by providing precise information about which asset caused the validation failure.
internal/connectors/plugins/public/wise/client/transfers.go (4)
12-12: Added import for error utilitiesAdded import for the error utilities package, which is used to improve error handling throughout the file.
84-88: Improved error handling with structured error wrappingReplaced direct error formatting with the
NewWrappedErrorutility, which provides a more structured approach to error handling. This change helps implement the PR objective of improving connector error handling by separating the cause from the full error stack trace.
175-179: Consistent error handling for GetTransfer methodApplied the same structured error handling pattern as in the GetTransfers method, maintaining consistency across the codebase.
205-209: Consistent error handling for CreateTransfer methodApplied the same structured error handling pattern as in other methods, ensuring a consistent approach to error handling throughout the client implementation.
internal/connectors/plugins/public/wise/client/webhooks.go (4)
10-11: Added import for error utilitiesAdded import for the error utilities package, which is used to implement consistent error handling throughout the file.
69-73: Improved error handling in CreateWebhook methodReplaced direct error formatting with
NewWrappedErrorutility, which provides better error structure and aligns with the PR objective of improving connector error handling.
88-92: Consistent error handling in ListWebhooksSubscription methodApplied the same structured error handling pattern as in the CreateWebhook method, maintaining consistency across the codebase.
106-110: Consistent error handling in DeleteWebhooks methodApplied the same structured error handling pattern as in other methods, ensuring a consistent approach to error handling throughout the webhook management functionality.
internal/utils/errors/errors_test.go (4)
1-9: Well-organized test file setupThe package declaration and imports are properly organized, including necessary testing utilities and error packages required for the tests.
11-24: Well-structured test function with parallel executionThe test function is set up to run in parallel with a clean structure for defining test cases. The testCase struct provides a clear template for defining different error scenarios to test.
25-56: Comprehensive test cases for error wrapping scenariosGood coverage of different error wrapping scenarios, including:
- Simple error wrapping
- Double error wrapping
- Interactions between logical errors and wrapped errors
- Standard error wrapping behavior
This ensures the error utilities work correctly in various contexts that might occur in the codebase.
58-71: Clear test execution with appropriate assertionsThe test execution logic is clean and well-organized, using parallel subtests and appropriate assertions to verify both error causes and error matching. The use of
require.Equalandrequire.ErrorIsensures proper validation of the error utilities' behavior.internal/connectors/plugins/public/moneycorp/transfers.go (2)
23-26: Improved error handling with structured wrapping.The change from using simple
fmt.Errorfto a dedicated error wrapping utility provides better error classification by marking this as an invalid request error, which likely enables better error handling at the API layer.
31-34: Enhanced error messaging with amount context.Good improvement to include the amount value in the error message, which will help with debugging invalid conversion issues while maintaining the proper error classification.
internal/connectors/plugins/public/wise/utils.go (1)
13-16: Standardized error handling with improved context.The validation errors now consistently include the context "in transfer/payout request" and use the structured error wrapping pattern. This makes error messages more descriptive and helps classify them as invalid request errors rather than internal errors.
Also applies to: 20-23, 28-31, 36-39, 44-47, 52-55
internal/connectors/plugins/public/bankingcircle/transfers.go (3)
16-19: Enhanced validation error handling.The improved error messages explicitly state the context "in transfer request" and use structured error wrapping for better error classification. This ensures API clients receive appropriate error types.
Also applies to: 23-26
56-59: Comprehensive error handling for account validation.The code now provides detailed error messages about source and destination account issues, including specific account references in the messages and proper error classification.
Also applies to: 62-65, 71-74, 77-80
39-42:❓ Verification inconclusive
Added test coverage needed for currency-related error paths.
The error handling for currency and amount parsing now uses structured wrapping, which is good. However, static analysis shows these lines aren't covered by tests.
Also applies to: 47-50
🏁 Script executed:
#!/bin/bash # Check if there are tests for the createTransfer function error paths rg -A 5 -B 5 "Test.*createTransfer.*Error" --type goLength of output: 54
Action Required: Please Add Test Coverage for Currency Error Handling
- The structured wrapping for error handling in file
internal/connectors/plugins/public/bankingcircle/transfers.go(lines 39–42 and 47–50) looks good.- However, our static analysis (using
rg "Test.*createTransfer.*Error") did not find any tests covering error paths for currency and amount parsing.- Please add test cases that specifically trigger these error conditions to verify that the correct error (
models.ErrInvalidRequest) is wrapped and returned.internal/connectors/plugins/public/mangopay/client/wallets.go (2)
42-47: Simplified HTTP error handling.The error handling has been revised to focus on the status code rather than parsing detailed error responses. This simplifies the error chain while still preserving the essential information.
62-67: Consistent error wrapping pattern for HTTP requests.The GetWallet method now follows the same error handling pattern as GetWallets, providing consistent error handling throughout the client implementation.
internal/connectors/plugins/public/currencycloud/transfers_test.go (3)
72-72: Improved error message with better context.The error message now explicitly mentions "in transfer request", which provides better context to the user about where the required field is missing.
85-85: Improved error message with better context.The error message now explicitly mentions "in transfer request", which provides better context to the user about where the required field is missing.
98-98: Enhanced error message with specific asset details.The error message now includes the specific asset code "HUF" that caused the error, making it easier to identify the problematic value.
internal/connectors/plugins/public/mangopay/webhooks_test.go (3)
151-151: Improved error message clarity.The error message has been changed from a technical reference to an environment variable ("STACK_PUBLIC_URL is not set") to a more user-friendly message that clearly states what's missing.
374-374: Enhanced error message with detailed parsing information.The error message now includes the specific parsing error details, which helps in identifying the exact issue with the Date parameter.
392-392: Improved error message specificity.The error message now includes the specific unsupported event type "TEST", making it easier to identify which event type caused the issue.
internal/connectors/plugins/registry/plugins.go (2)
34-45: Removed unused parameter from function call.The
providerparameter has been removed from thesetupConfigcall, simplifying the function invocation.
47-47: Simplified function signature by removing unused parameter.The
providerparameter has been removed from thesetupConfigfunction signature since it wasn't being used within the function. This is a good practice to avoid having unused parameters.internal/connectors/plugins/public/currencycloud/payouts_test.go (3)
72-72: Improved error message with better context.The error message now explicitly mentions "in payout request", which provides better context to the user about where the required field is missing.
85-85: Improved error message with better context.The error message now explicitly mentions "in payout request", which provides better context to the user about where the required field is missing.
98-98: Enhanced error message with specific asset details.The error message now includes the specific asset code "HUF" that caused the error, making it easier to identify the problematic value.
internal/connectors/plugins/public/currencycloud/client/contacts.go (2)
12-12: Good addition of error utils import.Adding the error utilities import is a good step towards standardizing error handling across the codebase.
50-54:❓ Verification inconclusive
Good implementation of structured error handling.
Using
NewWrappedErrorwithmodels.ErrInvalidRequestis an excellent way to ensure consistent error classification while preserving the specific context message.
🏁 Script executed:
#!/bin/bash # Verify how this error is handled in calling code rg -A 5 "ErrInvalidRequest.*no contact found" --type goLength of output: 57
Please verify that all callers properly handle the wrapped error
The implementation in
internal/connectors/plugins/public/currencycloud/client/contacts.go(lines 50–54) correctly useserrorsutils.NewWrappedErrorwithmodels.ErrInvalidRequestto provide a well-structured, context-rich error. However, our automated grep search for"ErrInvalidRequest.*no contact found"did not return any references in the calling code. This might indicate one of the following:
- The calling code does not explicitly check for this error type and instead relies on generic error handling.
- The error is handled indirectly or logged without pattern matching on the specific message.
Recommendations:
- Manually verify that the error propagation and handling in the downstream callers appropriately distinguish and process errors flagged with
models.ErrInvalidRequest.- Ensure that any unit or integration tests cover the scenario where “no contact found for account” errors are raised, so that the structured error wrapping is effectively utilized.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 50-53: internal/connectors/plugins/public/currencycloud/client/contacts.go#L50-L53
Added lines #L50 - L53 were not covered by testsinternal/connectors/plugins/public/mangopay/plugin.go (6)
14-14: Good addition of error utils import.Adding the error utilities import is a good step towards standardizing error handling across the codebase.
185-189: Good implementation of structured error handling for EventType validation.Using
NewWrappedErrorwithmodels.ErrInvalidRequestis an excellent way to ensure consistent error classification for missing query parameters.
192-196: Good implementation of structured error handling for ResourceID validation.This follows the same pattern as the EventType parameter check, maintaining consistency in error handling.
199-203: Good implementation of structured error handling for Date validation.This follows the same pattern as the previous parameter checks, maintaining consistency in error handling.
206-210: Well-structured error for Date parsing failure.The error message includes the original parsing error using
%w, which is helpful for debugging while still classifying it as an invalid request.
220-224:❓ Verification inconclusive
Good implementation of structured error handling for unsupported webhook type.
Using
NewWrappedErrorwithmodels.ErrInvalidRequestis consistent with the other error handling in this method.
🏁 Script executed:
#!/bin/bash # Check if there are tests for the TranslateWebhook method rg -A 5 "TestTranslateWebhook" --type goLength of output: 40
Review Verification: Structured Error Handling is Solid, but Test Coverage Needs Confirmation
- The error handling in
internal/connectors/plugins/public/mangopay/plugin.go(lines 220–224) is well-implemented; wrapping the error usingerrorsutils.NewWrappedErrorwithmodels.ErrInvalidRequestaligns with the existing patterns in the method.- However, our initial automated search for tests targeting
TranslateWebhookdid not return any results. Please manually verify that appropriate tests exist to cover this error handling scenario or add tests as needed.internal/connectors/plugins/public/wise/config.go (4)
11-11: Good addition of error utils import.Adding the error utilities import is a good step towards standardizing error handling across the codebase.
26-30: Good implementation of structured error handling for invalid public key.Using
NewWrappedErrorwithmodels.ErrInvalidConfigis an excellent way to ensure consistent error classification for configuration validation.
34-38: Well-structured error for public key parsing failure.The error message properly includes the original parsing error using
%w, which provides useful context while still classifying it correctly as a configuration error.
44-48: Good implementation of structured error handling for invalid public key type.Using
NewWrappedErrorwithmodels.ErrInvalidConfigmaintains consistency with the other error handling in this method.internal/utils/errors/errors.go (6)
1-4: Good package structure for error utilities.Creating a dedicated package for error handling is a good architectural decision to centralize and standardize error management across the codebase.
5-8: Simple and effective error wrapper structure.The
wrappedErrorstruct is minimal yet sufficient for the purpose of wrapping errors.
19-21: Simple implementation of Error method.The
Errormethod correctly returns the error message from the wrapped error.
28-49: Well-structured Cause function with good error traversal.The
Causefunction properly handles both single and multiple unwrapped errors, ensuring it gets to the root cause. The comment that the cause error is always the first one is a reasonable assumption.
23-26:❓ Verification inconclusive
Potential panic in Unwrap method.
The
Unwrapmethod assumes thate.errimplements theUnwrap() []errorinterface, which might not always be the case. Ife.errdoesn't implement this interface, it will cause a panic at runtime.Add a type assertion check before calling the method:
func (e *wrappedError) Unwrap() []error { - return e.err.(interface{ Unwrap() []error }).Unwrap() + if unwrapper, ok := e.err.(interface{ Unwrap() []error }); ok { + return unwrapper.Unwrap() + } + // Handle the case where e.err doesn't implement Unwrap() []error + if unwrapper, ok := e.err.(interface{ Unwrap() error }); ok { + if err := unwrapper.Unwrap(); err != nil { + return []error{err} + } + } + return []error{e.err} }
🏁 Script executed:
#!/bin/bash # Check if there are tests for the errors package rg -A 5 "TestNewWrappedError|TestUnwrap|TestCause" --type goLength of output: 62
Action Required: Verify Safety of Error Unwrap Implementation
The current implementation in
internal/utils/errors/errors.go:func (e *wrappedError) Unwrap() []error { return e.err.(interface{ Unwrap() []error }).Unwrap() }assumes that
e.errimplements the interface withUnwrap() []error, which may not always be true. This risks a runtime panic if that assumption fails. The suggested change adds type-checks for bothUnwrap() []errorandUnwrap() errorcases, providing safe fallbacks:func (e *wrappedError) Unwrap() []error { - return e.err.(interface{ Unwrap() []error }).Unwrap() + if unwrapper, ok := e.err.(interface{ Unwrap() []error }); ok { + return unwrapper.Unwrap() + } + // Handle the case where e.err doesn't implement Unwrap() []error + if unwrapper, ok := e.err.(interface{ Unwrap() error }); ok { + if err := unwrapper.Unwrap(); err != nil { + return []error{err} + } + } + return []error{e.err} }Next Steps:
- Manual/Additional Test Verification: No tests were found via our search for common error-related tests (e.g., “TestNewWrappedError”, “TestUnwrap”, “TestCause”). Please ensure that the updated logic is covered by appropriate tests or manually verify its behavior when
e.errdoes not implement either interface.- Impact Check: Double-check that this change does not adversely affect the usage of
errors.Isor other parts of the error handling system.
9-17:✅ Verification successful
Issue with multiple %w verbs in
NewWrappedError.The
fmt.Errorfcall uses two%wverbs, which is problematic becausefmt.Errorfonly supports one%wverb for wrapping errors. This could lead to unexpected behavior when unwrapping errors.Modify the implementation to use a different approach:
func NewWrappedError(cause error, newError error) error { return &wrappedError{ - err: fmt.Errorf("%w: %w", cause, newError), + err: fmt.Errorf("%s: %w", cause.Error(), newError), } }Or alternatively, if you want to preserve both errors for unwrapping:
type wrappedError struct { - err error + cause error + newError error } func NewWrappedError(cause error, newError error) error { return &wrappedError{ - err: fmt.Errorf("%w: %w", cause, newError), + cause: cause, + newError: newError, } } func (e *wrappedError) Error() string { - return e.err.Error() + return fmt.Sprintf("%s: %s", e.cause.Error(), e.newError.Error()) }
🌐 Web query:
Can fmt.Errorf handle multiple %w verbs?💡 Result:
Starting with Go 1.20,
fmt.Errorfsupports multiple%wverbs, enabling a single error to wrap multiple underlying errors. This enhancement simplifies error handling in scenarios where operations might generate several independent errors that need preservation.Key Points
Multiple Error Wrapping
Go 1.20 allows using multiple%wverbs infmt.Errorfto create an error that wraps a list of errors. For example:err1 := errors.New("file missing") err2 := errors.New("permission denied") wrapped := fmt.Errorf("operation failed: %w and %w", err1, err2)This captures both errors in
wrapped, allowing inspection viaerrors.Isanderrors.As[3][7].Backward Compatibility
Previous versions of Go restrictedfmt.Errorfto one%wverb per call, generating a compile-time error for multiple uses[6][9]. The 1.20 update removes this limitation, treating multiple%wverbs as a list of errors[1][7].Inspecting Wrapped Errors
errors.Is(wrapped, err1)anderrors.Is(wrapped, err2)will both returntrue[3][7].errors.Ascan target specific error types in the wrapped list[7][10].- However,
errors.Unwrap()returnsnilfor these composite errors, as they don’t implementUnwrap() []error[7].Practical Use Cases
Atomic Operations
When reverting a multi-step operation (e.g., moving files), multiple errors can be captured:if revertErr := os.Rename(dest, src); revertErr != nil { return fmt.Errorf("revert failed: %w; original error: %w", revertErr, err) }This preserves both the original and rollback errors[1][7].
Error Aggregation
Usefmt.Errorfwith%wto combine HTTP and database errors into a single error chain:var ( NotFoundHTTP = errors.New("404") DBErr = errors.New("database error") ) err := fmt.Errorf("request failed: %w (%w)", DBErr, NotFoundHTTP)This allows handlers to check for
NotFoundHTTPorDBErr[3][7].Limitations
- Composite errors from
fmt.Errorfwith multiple%wverbs cannot be directly unwrapped into a slice. For that, useerrors.Join(err1, err2), which returns an error withUnwrap() []error[7][10].- Custom error types implementing
Unwrap() []errorare required for granular iteration over individual errors[7].This update streamlines error handling in Go, especially for complex workflows where bundling multiple errors improves debuggability.
Citations:
- 1: https://www.tiredsg.dev/blog/wrapping-multiple-errors-golang/
- 2: https://blog.anynines.com/posts/error-handling-in-go-part-2/
- 3: https://lukas.zapletalovi.com/posts/2022/wrapping-multiple-errors/
- 4: https://groups.google.com/g/golang-nuts/c/-bYObQTWX_8/m/9iwZrQnIBAAJ
- 5: https://rollbar.com/blog/golang-wrap-and-unwrap-error/
- 6: cmd/vet: check against fmt.Errorf uses with more than one %w golang/go#34062
- 7: https://endorama.dev/2023/golang-1.20-multiple-errors/
- 8: Make
fmt.Errorfuse%wto wrap error instead of%vkubernetes/kubernetes#123234- 9: https://groups.google.com/g/golang-nuts/c/SwKZ-qJmZl0
- 10: https://pkg.go.dev/errors
Attention: Verify Go Version Compliance for Error Wrapping
Based on recent updates in Go 1.20,
fmt.Errorfnow supports multiple%wverbs. The current implementation ofNewWrappedErrorininternal/utils/errors/errors.gois therefore valid if your project is built with Go 1.20 or later.
- Action Items:
- Confirm that the project’s Go version (in
go.mod) is set to 1.20 or above.- If backward compatibility with pre-1.20 versions is required, consider revising the error wrapping strategy (e.g., using
%sfor one of the errors or adopting a custom error type for multiple unwrapping).No changes are needed for environments running Go 1.20 or later.
internal/connectors/plugins/public/moneycorp/payouts_test.go (3)
72-72: Enhanced error message with more contextThe error message has been improved to provide more specific context about where the source account is required ("in transfer/payout request"), making it clearer to API consumers why their request failed.
85-85: Enhanced error message with more contextSimilar to the source account error message, this destination account error now provides more specific context by adding "in transfer/payout request", improving clarity for users debugging their requests.
98-98: Improved error message with specific asset detailsThe error message now includes the specific unsupported asset ("HUF") that caused the failure, making troubleshooting more straightforward for users by explicitly identifying which currency is problematic.
internal/connectors/plugins/public/modulr/transfers_test.go (3)
72-72: Enhanced error message with more contextThe error message has been improved to provide more specific context about where the source account is required ("in transfer/payout request"), making it clearer to API consumers why their request failed.
85-85: Enhanced error message with more contextSimilar to the source account error message, this destination account error now provides more specific context by adding "in transfer/payout request", improving clarity for users debugging their requests.
98-98: Improved error message with specific asset detailsThe error message now includes the specific unsupported asset ("HUF") that caused the failure, making troubleshooting more straightforward for users by explicitly identifying which currency is problematic.
internal/connectors/plugins/public/moneycorp/client/balances.go (4)
10-10: New error utilities import addedAdded import for the error utilities package that provides structured error handling functions.
42-43: Improved error type and response handlingChanged from
moneycorpErrortomoneycorpErrorstype and now capturing the HTTP status code separately, which allows for more specific error handling based on the status code.
45-48: Direct status code check instead of error property checkNow checking the HTTP status code directly rather than a property of the error response, which is a more reliable approach for detecting specific HTTP errors like 404 Not Found.
49-53: Structured error wrapping with better contextReplaced direct error return with
errorsutils.NewWrappedErrorwhich provides better error context while preserving the original error chain. This aligns with the PR's goal of improving error messages while reducing unnecessary stack trace information.internal/connectors/plugins/public/mangopay/client/payin.go (2)
9-9: Updated error utilities importChanged the import to use the internal error utilities package that provides structured error handling functions.
44-47: Improved error handling with status code contextReplaced direct error return with
errorsutils.NewWrappedErrorwhich now includes the HTTP status code in the error message. This provides more diagnostic information while maintaining the original error chain, aligning with the PR's goal of improving error clarity.internal/connectors/plugins/public/currencycloud/payouts.go (5)
11-11: Good addition of errorsutils import.The addition of the error utilities package is consistent with the PR's goal of improving error handling.
16-19: Good error handling improvement.The change from direct error returns to using
errorsutils.NewWrappedErrorprovides a more structured approach for error handling, aligning with the PR's objective to improve connector errors.
23-26: Consistent error wrapping pattern applied.The error handling pattern is consistently applied across different validation checks, which improves code readability and error tracing.
39-42: Improved error context provided.The error message now clearly indicates what operation failed (getting currency and precision from asset) while maintaining the appropriate error type (ErrInvalidRequest).
47-50: Enhanced error message with relevant context.The error now includes the amount value that failed conversion, providing better debugging context while maintaining the appropriate error type.
internal/connectors/plugins/public/mangopay/transfers.go (4)
13-13: Good addition of errorsutils import.The addition of the error utilities package is consistent with the PR's goal of improving error handling.
20-23: Improved error message with specific reference value.The error now includes the problematic reference value in the message, which makes debugging easier by specifying exactly what value was invalid.
27-30: Consistent error wrapping approach applied to all validation checks.All validation error cases now use the same structured approach with
errorsutils.NewWrappedError, making the code more maintainable and consistent.Also applies to: 34-37, 42-45
60-63: Standardized error handling for asset validation.The error handling for currency/precision retrieval now matches the pattern used throughout the codebase, improving consistency.
internal/connectors/plugins/public/modulr/transfers.go (3)
12-12: Good addition of errorsutils import.The addition of the error utilities package is consistent with the PR's goal of improving error handling.
22-25: Proper Go error wrapping with %w verb.Using the
%wverb infmt.Errorfis the recommended Go approach for proper error unwrapping. This enables the wrapped error to be examined later witherrors.Unwrap.
30-33: Includes amount in error message for better debugging.The error message now includes the actual amount value that failed conversion, providing valuable context for debugging while maintaining proper error wrapping with
%w.internal/connectors/plugins/public/mangopay/client/webhooks.go (4)
11-11: Updated import path for errorsutils.The import path has been updated to use the internal package rather than the external one, which helps with maintaining control over the error utilities functionality.
102-106: Improved error handling with status code context.The error now includes the HTTP status code in the message, which is valuable context for diagnosing API communication issues.
136-140: Enhanced error reporting with error details.The error message now includes the specific error from the error response, providing more detailed context about what went wrong during hook creation.
170-174: Consistent error handling pattern applied.The same structured error wrapping approach is consistently applied to all API operations, improving code maintainability and error tracing capabilities.
internal/connectors/plugins/public/modulr/utils.go (4)
8-8: Import of error utilities package added correctly.The error utilities package is imported with a clear alias, which is consistent with the changes being made to error handling in this file.
17-20: Improved error handling for source account validation.The code now uses
errorsutils.NewWrappedErrorinstead of directfmt.Errorfwith%w, which aligns with the PR objective of enhancing error handling. The error message is also more descriptive by specifying this is related to a "transfer/payout request".
24-27: Improved error handling for destination account validation.Similar to the source account validation, this change uses the structured error wrapping approach with a more descriptive error message, maintaining consistency across the validation checks.
31-34: Enhanced error handling for description validation.The error message is now more informative by including the actual validation requirements (character limit and regex pattern) along with the invalid value. This will help users understand exactly why their input was rejected.
internal/connectors/plugins/public/wise/client/payouts.go (3)
12-12: Error utilities package import added properly.The import with a clear alias is consistent with the changes in error handling being implemented in this file.
78-82: Improved error handling in GetPayout method.The error handling now uses
errorsutils.NewWrappedErrorto create a more structured error that includes both the context message and the original error. This aligns with the PR objective of enhancing error handling in connector operations.
108-112: Consistent error handling in CreatePayout method.This change follows the same pattern as in the GetPayout method, using
errorsutils.NewWrappedErrorto provide better context for errors. The consistent approach across methods improves the overall error handling structure in the codebase.internal/connectors/plugins/public/mangopay/client/transfer.go (3)
11-11: Updated import path for error utilities.The error utilities import path has been updated to use the one from the payments package, which ensures consistency with the rest of the codebase.
66-72: Enhanced error handling in InitiateWalletTransfer.The code now discards the status code and uses
errorsutils.NewWrappedErrorto provide a more structured error message. This change aligns with the PR objective of not storing the entire stack trace for transfer operations and improving error messaging.
89-92: Improved error handling in GetWalletTransfer.The error handling now uses
errorsutils.NewWrappedErrorfor consistent error reporting. The status code is still included in the error message for context, but the error structure is improved.internal/connectors/plugins/public/currencycloud/transfers.go (5)
11-11: Error utilities package import added correctly.The error utilities package is imported with a clear alias, consistent with the changes in error handling being implemented.
16-20: Enhanced error handling for source account validation.The code now uses
errorsutils.NewWrappedErrorto create a better structured error that explicitly marks this as an invalid request. The error message now clearly specifies this is related to a "transfer request", providing better context for the user.
23-27: Improved error handling for destination account validation.Similar to the source account validation, this change uses the structured error wrapping approach with a clearer error message, maintaining consistency across validation checks.
39-43: Enhanced error handling for currency and precision retrieval.The error is now properly wrapped using
errorsutils.NewWrappedError, maintaining the original error while adding context. This ensures that the user receives aBadRequestresponse instead of an internal server error when providing an invalid asset.
47-51: Improved error handling for amount conversion.The error message now includes both the original amount and the conversion error, providing better context for debugging. The use of
errorsutils.NewWrappedErrorensures consistent error handling throughout the function.internal/connectors/plugins/public/modulr/payouts_test.go (3)
72-72: Improved error message specificity.The error message now clearly indicates the context (transfer/payout request) where the source account is required, making it more informative for debugging and user understanding.
85-85: Improved error message specificity.Similar to the previous change, adding the transfer/payout request context to the error message makes it more descriptive and helps users understand exactly where the destination account is required.
98-98: Enhanced error detail by including currency code.The updated error message now includes the specific currency code (HUF) that's causing the failure, making it easier to diagnose the exact cause of the validation error.
internal/connectors/plugins/public/wise/client/recipient_accounts.go (3)
8-9: Added error utilities import.The new import enables the use of structured error wrapping.
52-56: Improved error wrapping structure.Replaced direct error formatting with structured error wrapping, which provides better error context hierarchy and will enable proper error cause extraction downstream.
This change aligns with the PR objective of improving connector error handling.
85-89: Improved error wrapping structure.Similar to the previous change, this implements structured error wrapping for better error handling in the GetRecipientAccount method.
The new approach separates the contextual message from the original error, which will improve error classification in the API layer.
internal/connectors/plugins/public/atlar/client/client.go (4)
12-12: Added error utilities import.Added import for the error utilities package that provides structured error wrapping.
108-108: Improved error wrapping for client errors.Implemented structured error wrapping for client-side HTTP errors (4xx), which supports the PR objective of improving how BadRequest errors are handled.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 108-108: internal/connectors/plugins/public/atlar/client/client.go#L108
Added line #L108 was not covered by tests
110-110: Improved error wrapping for server errors.Implemented structured error wrapping for server-side HTTP errors (5xx), which ensures consistent error handling across different error types.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 110-110: internal/connectors/plugins/public/atlar/client/client.go#L110
Added line #L110 was not covered by tests
104-104:❓ Verification inconclusive
Improved error wrapping for rate limiting errors.
Replaced direct error formatting with structured error wrapping for rate limiting errors, enabling better error cause extraction.
The static analysis shows this line is not covered by tests. You might want to add tests for this error handling path:
🏁 Script executed:
#!/bin/bash # Check test coverage for error handling in the wrapSDKErr function fd "wrapSDKErr.*_test.go" | xargs catLength of output: 39
Attention: Missing Test Coverage for Enhanced Error Wrapping in Client
The improvements to error wrapping for rate limiting errors are clear and beneficial. However, the static analysis indicates that the new error handling path (specifically in the
wrapSDKErrfunction and at line 104 ininternal/connectors/plugins/public/atlar/client/client.go) is not covered by any tests. The test search command produced no output, suggesting that no tests exist for this functionality.
- Verify and add tests to cover the error path starting at line 104 in
internal/connectors/plugins/public/atlar/client/client.go.- Ensure that the new structured error wrapping is validated in tests to enable reliable error cause extraction.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 104-104: internal/connectors/plugins/public/atlar/client/client.go#L104
Added line #L104 was not covered by testsinternal/connectors/plugins/public/mangopay/transfers_test.go (5)
76-76: Enhanced error message with specific reference value.The error message now includes both the specific reference value ("test") and the context ("transfer request"), making it much more informative for debugging.
89-89: Improved error message specificity.The error message now clarifies that the source account is required specifically in the context of a transfer request.
102-102: Improved error message specificity.The error message now clarifies that the destination account is required specifically in the context of a transfer request.
115-115: Improved error message specificity.The error message now specifies that the user ID in source account metadata is required specifically in the context of a transfer request.
128-128: Enhanced error detail by including currency code.The updated error message now includes the specific currency code (HUF) that's causing the failure, making it easier to diagnose the exact cause of the validation error.
internal/connectors/plugins/public/bankingcircle/payouts_test.go (7)
102-102: Improved error message clarity.The error message now includes "in payout request" context, making it clearer which operation failed.
115-115: Improved error message clarity.The error message now includes "in payout request" context, making it clearer which operation failed.
128-128: Improved error message with specific asset information.The error message now includes the specific currency "HUF" that caused the issue, making troubleshooting easier.
139-139: Improved error message clarity.The error message now includes "in payout request" context, making it clearer which operation failed.
151-151: Improved error message clarity.The error message now includes "in payout request" context, making it clearer which operation failed.
165-165: Improved error message with account reference.The error message now includes the specific account reference "acc1", making it easier to identify the problematic account.
181-181: Improved error message with account reference.The error message now includes the specific account reference "acc1", making it easier to identify the problematic account.
internal/connectors/plugins/public/mangopay/payouts_test.go (6)
76-76: Improved error message with request context.The error message now specifies that the UUID reference is required specifically "in payout request", providing better context.
89-89: Improved error message clarity.The error message now includes "in payout request" context, making it clearer which operation failed.
102-102: Improved error message clarity.The error message now includes "in payout request" context, making it clearer which operation failed.
115-115: Improved error message with request context.The error message now clarifies that the user ID metadata is required "in payout request", providing better context for troubleshooting.
128-128: Improved error message with request context.The error message now specifies that the description format requirement is specifically "in payout request", providing better context.
141-141: Improved error message with specific asset information.The error message now includes the specific currency "HUF" that caused the issue, making troubleshooting easier.
internal/connectors/plugins/public/bankingcircle/transfers_test.go (7)
72-72: Improved error message clarity.The error message now includes "in transfer request" context, making it clearer which operation failed.
85-85: Improved error message clarity.The error message now includes "in transfer request" context, making it clearer which operation failed.
98-98: Improved error message with specific asset information.The error message now includes the specific currency "HUF" that caused the issue, making troubleshooting easier.
112-112: Improved error message with account reference.The error message now includes the specific account reference "acc1", making it easier to identify the problematic account.
128-128: Improved error message with account reference.The error message now includes the specific account reference "acc1", making it easier to identify the problematic account.
147-147: Improved error message with account reference.The error message now includes the specific account reference "acc2", making it easier to identify the problematic account.
168-168: Improved error message with account reference.The error message now includes the specific account reference "acc2", making it easier to identify the problematic account.
internal/connectors/plugins/public/wise/payouts_test.go (7)
79-79: Improved error message clarity.The error message now includes "in transfer/payout request" context, making it clearer which operation failed.
92-92: Improved error message clarity.The error message now includes "in transfer/payout request" context, making it clearer which operation failed.
105-105: Improved error message with request context.The error message now clarifies that the profile ID metadata is required "in transfer/payout request", providing better context for troubleshooting.
118-118: Improved error message with request context.The error message now clarifies that the profile ID must be an integer "in transfer/payout request", providing better context for troubleshooting.
131-131: Improved error message with request context.The error message now clarifies that the destination account profile ID is required "in transfer/payout request", providing better context for troubleshooting.
144-144: Improved error message with request context.The error message now clarifies that the destination profile ID must be an integer "in transfer/payout request", providing better context for troubleshooting.
157-157: Improved error message with specific asset information.The error message now includes the specific currency "HUF" that caused the issue, making troubleshooting easier.
internal/connectors/plugins/public/wise/transfers_test.go (1)
79-79: Enhanced error messages with better contextThe error messages have been improved by adding "in transfer/payout request" to provide better context about where the validation failure occurred. This makes the error messages more descriptive and user-friendly, which aligns with the PR's goal of improving connector error messaging.
Also applies to: 92-92, 105-105, 118-118, 131-131, 144-144, 157-157
internal/api/services/errors.go (1)
7-7: Improved error handling with dedicated utilitiesGreat improvement to use
errorsutils.NewWrappedErrorinstead offmt.Errorffor wrapping errors. This change standardizes error handling across the codebase and aligns with the PR's goal of improving error handling.Also applies to: 51-54
internal/connectors/plugins/public/atlar/client/counter_parties.go (1)
10-10:❓ Verification inconclusive
Improved error wrapping with dedicated utilities
Using
errorsutils.NewWrappedErrorinstead of direct error wrapping withfmt.Errorfis a good improvement that aligns with the PR's goal of standardizing error handling.The static analysis tool indicates that these lines are not covered by tests. Consider adding a test case for this error scenario:
Also applies to: 70-73
🏁 Script executed:
#!/bin/bash # Check if there are tests covering the error case when a counterparty is not created with exactly one account # Look for relevant test files rg --type go -l "PostV1CounterParties" --glob "**/*_test.go" # Look for test cases that might be testing this error condition rg --type go -A 5 -B 5 "counterparty was not created with exactly one account" --glob "**/*_test.go"Length of output: 234
Attention: Missing Test Coverage for Error Scenario
- The update to use
errorsutils.NewWrappedErrorfor improved error wrapping is a good step toward standardizing error handling.- However, our static analysis indicates that the behavior on line 10 (and similarly on lines 70-73) in
internal/connectors/plugins/public/atlar/client/counter_parties.gois not explicitly covered by tests.- The search revealed one related test file (
internal/connectors/plugins/public/atlar/bank_account_creation_test.go), but no test case appears to target the error condition "counterparty was not created with exactly one account".Please confirm whether this scenario is indirectly tested or consider adding a dedicated test to verify the error handling for cases where a counterparty does not have exactly one account.
internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go (2)
12-12: Import added for new error handlingThe import for the error utilities package is added to support the improved error handling mechanism.
60-64:✅ Verification successful
Improved error handling with structured wrapping
The error handling has been improved by using
errorsutils.NewWrappedErrorinstead of concatenating error messages withfmt.Errorf. This approach properly separates the context message from the original error while preserving both.
🏁 Script executed:
#!/bin/bash # Verify test coverage for this error handling code echo "Checking test coverage for transfer_payouts.go file..." find . -name "*_test.go" | grep -i "transfer_payouts" || echo "No test file found for transfer_payouts.go" # Look for any tests that might cover this specific function echo "Searching for tests that might cover InitiateTransferOrPayouts function..." rg -l "InitiateTransferOrPayouts" --type go | grep "_test.go" || echo "No tests found covering InitiateTransferOrPayouts"Length of output: 618
Verified: Improved Error Handling with Structured Wrapping
The updated error handling in
internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.gonow leverageserrorsutils.NewWrappedErrorto clearly separate context information from the underlying error. This change enhances readability and maintainability.
- Although a dedicated test file for
transfer_payouts.gowas not found, related tests covering theInitiateTransferOrPayoutsfunctionality exist in:
internal/connectors/plugins/public/bankingcircle/payouts_test.gointernal/connectors/plugins/public/bankingcircle/transfers_test.go- These tests help ensure that the error wrapping logic is properly exercised in practice. If there are any unique edge cases specific to
transfer_payouts.go, please make sure they are covered within your integration tests.Overall, the improvements are solid and no further changes are required at this time.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 60-63: internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go#L60-L63
Added lines #L60 - L63 were not covered by testsinternal/connectors/plugins/public/bankingcircle/client/accounts.go (3)
11-11: Import added for new error handlingThe import for the error utilities package is added to support the improved error handling mechanism.
98-102:✅ Verification successful
Improved error handling in GetAccount method
Similar to the GetAccounts method, the error handling in GetAccount has been improved to use structured error wrapping, which maintains a consistent approach to error handling throughout the codebase.
🏁 Script executed:
#!/bin/bash # Verify test coverage for the GetAccount method error handling echo "Searching for tests that might cover GetAccount function..." rg -l "GetAccount[^s]" --type go | grep "_test.go" || echo "No tests found covering GetAccount"Length of output: 587
Action Required: Confirmed consistent error-wrapping in GetAccount
The updated error handling in the GetAccount method now consistently uses structured error wrapping (via
errorsutils.NewWrappedError), aligning with the approach taken in GetAccounts. Our test coverage verification (notably intest/e2e/api_accounts_test.goand related bankingcircle tests) indicates that the changes are adequately exercised.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 98-101: internal/connectors/plugins/public/bankingcircle/client/accounts.go#L98-L101
Added lines #L98 - L101 were not covered by tests
75-79:✅ Verification successful
Improved error handling in GetAccounts method
The error handling has been enhanced by using
errorsutils.NewWrappedErrorto properly wrap the original error with a contextual message, making the error structure more consistent with the rest of the codebase.
🏁 Script executed:
#!/bin/bash # Verify test coverage for the GetAccounts method error handling echo "Checking test coverage for accounts.go file..." find . -name "*_test.go" | grep -i "accounts" || echo "No test file found for accounts.go" # Look for any tests that might cover the GetAccounts function echo "Searching for tests that might cover GetAccounts function..." rg -l "GetAccounts" --type go | grep "_test.go" || echo "No tests found covering GetAccounts"Length of output: 3706
Action: Confirm Improved Error Handling in GetAccounts Method
The updated error handling in the
GetAccountsmethod now wraps the original error usingerrorsutils.NewWrappedError, which provides better context and aligns with our codebase practices. The presence of tests—particularly ininternal/connectors/plugins/public/bankingcircle/accounts_test.go—confirms that these changes are adequately covered.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 75-78: internal/connectors/plugins/public/bankingcircle/client/accounts.go#L75-L78
Added lines #L75 - L78 were not covered by testsinternal/connectors/plugins/public/bankingcircle/client/client.go (2)
13-13: Import added for new error handlingThe import for the error utilities package is added to support the improved error handling mechanism.
47-51: Enhanced error handling with specific error typeThe error handling has been improved by using
errorsutils.NewWrappedErrorto wrap the original error with both a contextual message and the specific error typemodels.ErrInvalidConfig. This allows callers to better identify and handle configuration errors.internal/connectors/engine/errors.go (1)
3-8: Added imports for new error handling functionThese imports support the new
handleWorkflowErrorfunction, bringing in necessary dependencies for Temporal workflow error handling and the utility error functions.internal/connectors/plugins/public/atlar/payouts.go (2)
11-11: Good addition of the errors utility package.Adding the error utilities package aligns with the PR's goal of improving connector error handling.
22-25: Enhanced error handling with proper error categorization.This change improves error handling by explicitly wrapping errors with
models.ErrInvalidRequest, which will allow the API to respond with a BadRequest status instead of an internal server error when currency validation fails. This aligns with the PR objective to improve connector error handling.internal/connectors/plugins/public/stripe/create_payouts.go (2)
14-14: Good addition of the errors utility package.Adding the error utilities package is consistent with the PR's objective to improve connector error handling.
25-28: Enhanced error handling with proper error categorization.The change improves error handling by using
errorsutils.NewWrappedErrorto wrap currency validation errors withmodels.ErrInvalidRequest. This ensures user input errors are properly categorized and presented as BadRequest errors rather than internal server errors, which aligns with the PR objective.internal/connectors/plugins/public/currencycloud/client/accounts.go (2)
10-10: Good addition of the errors utility package.Adding the error utilities package is consistent with the PR's objective to improve connector error handling.
53-57:❓ Verification inconclusive
Improved error wrapping with more consistent error structure.
This change enhances error handling by using
errorsutils.NewWrappedErrorto create a more structured error that maintains the original error as the cause while providing a clear context message.The changes to these lines are not covered by tests according to static analysis. Consider adding unit tests to verify the error handling behavior:
🏁 Script executed:
#!/bin/bash # Check for test files that cover this code fd --type f "accounts_test.go" --exec grep -l "GetAccounts" {} \;Length of output: 388
Improved error wrapping with a more consistent error structure.
The updated error handling in
internal/connectors/plugins/public/currencycloud/client/accounts.gonow useserrorsutils.NewWrappedErrorto wrap the original error with clear contextual information. While a test file exists for the currencycloud connector (see./internal/connectors/plugins/public/currencycloud/accounts_test.go, as well as similar files in other connectors), it isn’t immediately clear that the specific error-wrapping branch inGetAccountsis fully exercised.
- Action Required:
Please verify manually that the test(s) ininternal/connectors/plugins/public/currencycloud/accounts_test.goexplicitly cover the error-wrapping behavior forGetAccounts. If not, consider adding or updating unit tests to precisely validate that scenario.#!/bin/bash # Example command to locate tests referencing GetAccounts fd --type f "accounts_test.go" --exec grep -l "GetAccounts" {} \;🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 53-56: internal/connectors/plugins/public/currencycloud/client/accounts.go#L53-L56
Added lines #L53 - L56 were not covered by testsinternal/connectors/plugins/public/wise/client/balances.go (3)
11-11: Good addition of the errors utility package.Adding the error utilities package is consistent with the PR's objective to improve connector error handling.
55-59: Improved error wrapping with more consistent error structure.This change enhances error handling by using
errorsutils.NewWrappedErrorto create a more structured error. The error now maintains the original error as the cause while providing a clear context message from the server response.
76-80: Improved error wrapping with more consistent error structure.This change enhances error handling by using
errorsutils.NewWrappedErrorto create a more structured error. The error now maintains the original error as the cause while providing a clear context message from the server response.internal/connectors/plugins/public/modulr/client/error.go (1)
8-9: Good addition of the modulrErrors typeThis introduces a collection type for errors, which is a good design choice for handling multiple errors from the API response.
internal/connectors/plugins/public/moneycorp/client/payouts.go (3)
11-11: Good addition of the errors utility packageAdding the dedicated errors utility package is aligned with the PR objective to improve error handling.
84-84: Appropriate update to error typeUpdating from
moneycorpErrortomoneycorpErrorsis consistent with the changes in the error handling mechanism.
87-91: Improved error handling and messagingThe use of
errorsutils.NewWrappedErrorprovides better structured error handling. Also, the message "failed to initiate payout" is more accurate than the previous "failed to initiate transfer".internal/connectors/plugins/public/modulr/client/accounts.go (5)
11-11: Good addition of the errors utility packageAdding the dedicated errors utility package is aligned with the PR objective to improve error handling.
50-50: Appropriate update to error type in GetAccountsUpdating from
modulrErrortomodulrErrorsis consistent with the changes in the error handling mechanism.
53-57: Improved error handling structure in GetAccountsThe use of
errorsutils.NewWrappedErrorprovides better structured error handling with clear separation between error context and the original error.
70-70: Appropriate update to error type in GetAccountUpdating from
modulrErrortomodulrErrorsis consistent with the changes in the error handling mechanism.
73-77: Improved error handling structure in GetAccountThe use of
errorsutils.NewWrappedErrorprovides better structured error handling with clear separation between error context and the original error.internal/connectors/plugins/public/modulr/client/transactions.go (3)
12-12: Good addition of the errors utility packageAdding the dedicated errors utility package is aligned with the PR objective to improve error handling.
46-46: Appropriate update to error typeUpdating from
modulrErrortomodulrErrorsis consistent with the changes in the error handling mechanism.
49-53: Improved error handling structureThe use of
errorsutils.NewWrappedErrorprovides better structured error handling with clear separation between error context and the original error.internal/connectors/engine/workflow/create_transfer.go (2)
8-8: Good addition of errorutils package for error handling improvement.Adding this import will allow proper error unwrapping, which is consistent with the PR objective to improve connector error handling.
194-207: Well implemented error cause extraction.This change improves error handling by unwrapping the error with
errorsutils.Cause, which prevents storing entire stack traces in the database. The comments clearly explain the rationale, and the implementation correctly uses the unwrapped error in the adjustment while preserving the original error flow in the return statement.internal/connectors/plugins/public/modulr/client/beneficiaries.go (2)
11-11: Good addition of errorutils package for standardized error wrapping.Adding this import supports the PR's goal of improving error handling throughout the codebase.
37-44: Improved error handling with structured wrapping.This change enhances error reporting by:
- Using
modulrErrorsinstead ofmodulrError, which likely provides more detailed error information- Using
errorsutils.NewWrappedErrorto create a structured error with cause chain- Preserving the same error message format for consistency
This implementation aligns with the PR objective to improve error messaging for connectors.
internal/api/v2/errors.go (2)
11-11: Good addition of errorutils package for error unwrapping.Adding this import enables extraction of the underlying error cause for validation errors.
31-32: Improved API error response for validation errors.This change enhances the API response for validation errors by:
- Extracting the underlying cause of the error with
errorsutils.Cause(err)- Passing only the cause to the
BadRequestresponseThis directly supports the PR objective to send a
BadRequestresponse with clearer error details when a request fails due to user input, rather than exposing internal error structures.internal/connectors/plugins/public/moneycorp/client/error.go (3)
11-13: Added Error method to moneycorpErrors struct.This addition allows the
moneycorpErrorsstruct to conform to the error interface, making error handling more consistent and standardized.
16-20: Standardized field name from StatusCode to Status.Renaming this field from
StatusCodetoStatusmakes the struct more consistent with standard error handling patterns. Make sure this change is consistently applied across all code that uses this struct.
25-28: Updated error handling to use Status field.The implementation correctly updates all references from
StatusCodetoStatusto reflect the field name change. This is part of standardizing error handling across connectors.Also applies to: 36-37, 41-45
internal/connectors/plugins/currency/currency.go (2)
208-208: Error message improvement: Included asset name in error message.The error message now includes the specific asset name that caused the error, providing more context to help with debugging and error resolution.
217-217: Error message improvement: Added invalid asset details.The error message now includes the actual asset string that was invalid, making it easier to diagnose issues when troubleshooting.
internal/connectors/plugins/public/moneycorp/client/accounts.go (3)
10-10: Added errorsutils import for improved error handling.The addition of the errorsutils package aligns with the PR objective to improve connector error handling.
46-50: Improved error handling with structured wrapping.The implementation now uses
errorsutils.NewWrappedErrorto create a more structured error with clear separation between the error message and the original error. This approach:
- Provides a clearer error message that includes the moneycorp error details
- Preserves the original error context
- Enables better error unwrapping downstream
This aligns with the PR objective to improve connector error handling.
43-43:✅ Verification successful
Changed error response type from moneycorpError to moneycorpErrors.
This suggests a change in the error structure to potentially handle multiple errors or provide more detailed error information.
🏁 Script executed:
#!/bin/bash # Check definition of both error types to understand the structure change rg -A 10 "type moneycorp(Error|Errors)" --type go internal/connectors/plugins/public/moneycorp/Length of output: 1703
Verified error type update:
After reviewing the definitions ininternal/connectors/plugins/public/moneycorp/client/error.go, we confirmed that the change frommoneycorpErrortomoneycorpErrorsis correct. ThemoneycorpErrorsstruct now properly encapsulates multiplemoneycorpErrorinstances and includes the necessary error handling method. The usage inaccounts.goat line 43 reflects this update properly.
- Confirmed definition changes in
error.goformoneycorpErrorsandmoneycorpError.- The code's current use of
moneycorpErrorsaligns with its new multi-error handling design.internal/connectors/plugins/public/moneycorp/client/recipients.go (3)
10-10: Added errorsutils import for improved error handling.The addition of the errorsutils package aligns with the PR objective to improve connector error handling.
46-46: Changed error response type from moneycorpError to moneycorpErrors.This suggests a change in the error structure to potentially handle multiple errors or provide more detailed error information, consistent with the change in accounts.go.
49-53: Improved error handling with structured wrapping.The implementation now uses
errorsutils.NewWrappedErrorto create a more structured error with clear separation between the error message and the original error, consistent with the improvement in accounts.go.internal/connectors/engine/workflow/create_payout.go (4)
8-8: Added errorsutils import for improved error handling.This import supports the error handling improvements in this file, aligning with the PR objective to improve connector error handling.
192-195: Added clear explanatory comments about error handling approach.The comments effectively explain the rationale behind unwrapping Temporal errors: to avoid storing the entire stack trace in the database. This improves code maintainability and helps future developers understand the design decisions.
195-195: Extract root cause from errors to avoid storing stack traces.Using
errorsutils.Cause()ensures only the root cause of the error is extracted, which directly addresses the PR objective of not storing entire stack traces in the database for transfer or payout creation errors.
205-205: Store only error cause in payment initiation adjustment.Changed from storing the full error with stack trace (
errPlugin) to storing only the root cause (cause). This helps keep database error records concise and focused on the essential error information.internal/connectors/engine/activities/errors.go (3)
11-11: Well-structured import addition for error utilities.The addition of the errorsutils import enables the improved error handling strategy throughout this file.
33-36: Good documentation on the error handling strategy.This comment clearly explains the rationale for extracting only the cause of the error when returning from Temporal activities, which helps maintain clarity for future developers.
41-41:❓ Verification inconclusive
Consistent implementation of error cause extraction.
The consistent use of
causeinstead of the full error object across all error paths effectively implements the stated goal of only passing the root cause to Temporal, which avoids storing unnecessary stack traces.Some modified lines aren't covered by tests according to codecov:
Also applies to: 43-43, 45-45, 47-47, 49-49, 61-61, 73-73, 75-75
🏁 Script executed:
#!/bin/bash # Check test coverage for the updated error handling # Find tests that exercise the temporalPluginErrorCheck function rg -l "temporalPluginErrorCheck" --type go --glob "*_test.go"Length of output: 61
Attention: Verify Test Coverage for Error Handling
The changes to extract and pass only the root cause are consistent with our design goals. However, our test search (usingrgfortemporalPluginErrorCheck) returned no results, which suggests that several modified lines ininternal/connectors/engine/activities/errors.go(lines 41, 43, 45, 47, 49, 61, 73, and 75) may not be directly exercised by our tests. Please verify manually whether these error paths are covered indirectly by other tests or consider adding explicit tests to ensure comprehensive coverage.internal/connectors/plugins/public/modulr/client/payments.go (2)
61-61: Type change from singular to plural error handling.Changing from
modulrErrortomodulrErrorssuggests an improvement in handling multiple error responses from the Modulr API.
64-67: Improved error wrapping with structured approach.The use of
errorsutils.NewWrappedErrorprovides a more consistent and structured way to combine API errors with the HTTP error context, improving debugging capabilities.internal/connectors/plugins/public/stripe/create_transfers_test.go (2)
77-77: More specific error message for missing destination account.The error message now clarifies that the destination account is required specifically "in transfer/payout request", providing better context for troubleshooting.
90-90: Enhanced error message with specific asset identifier.Adding the specific asset identifier "HHH" to the error message makes it easier to troubleshoot issues with unsupported assets by clearly identifying the problematic asset.
internal/connectors/plugins/public/adyen/client/client.go (2)
75-75:❓ Verification inconclusive
Consistent error wrapping for rate limiting errors.
Using
errorsutils.NewWrappedErrorfor rate limiting errors (HTTP 429) creates a more consistent error handling approach while preserving the original error context.This line isn't covered by tests according to codecov:
🏁 Script executed:
#!/bin/bash # Check test coverage for the rate limit handling # Find tests for the wrapSDKError function rg -l "wrapSDKError" --type go --glob "*_test.go" # Check if there are tests that specifically test rate limiting rg "StatusTooManyRequests" --type go --glob "*_test.go"Length of output: 105
Verification Pending: Test Coverage for Rate Limiting Error Handling
The use of
errorsutils.NewWrappedError(err, httpwrapper.ErrStatusCodeTooManyRequests)at line 75 ininternal/connectors/plugins/public/adyen/client/client.goappears consistent with our approach to error wrapping for rate limiting (HTTP 429). However, our initial script executions did not reveal any test files referencing either thewrapSDKErrorfunction or theStatusTooManyRequestserror, which suggests that this code path might not be explicitly covered by tests.
- Action Item: Please verify manually whether there are tests covering rate limiting error handling. If these tests exist but are not being detected by the grep scripts, consider updating the test identifiers. Otherwise, it might be necessary to add tests to ensure robust coverage for this error handling scenario.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 75-75: internal/connectors/plugins/public/adyen/client/client.go#L75
Added line #L75 was not covered by tests
79-79:❓ Verification inconclusive
Standardized error wrapping for client errors.
Using
errorsutils.NewWrappedErrorfor client errors (HTTP 400-499) provides a consistent approach to error handling, making error propagation more predictable throughout the codebase.This line isn't covered by tests according to codecov:
🏁 Script executed:
#!/bin/bash # Check test coverage for client error handling # Find tests for HTTP client errors rg "StatusBadRequest" --type go --glob "*_test.go"Length of output: 11623
Standardized Client Error Wrapping – Please Verify Test Coverage
The use of
errorsutils.NewWrappedError(err, httpwrapper.ErrStatusCodeClientError)at line 79 ininternal/connectors/plugins/public/adyen/client/client.goadheres to our error-handling convention for HTTP 400–499 errors by ensuring a consistent propagation of client error details.Although our broader test suite (as indicated by multiple references to
StatusBadRequest) confirms that client error cases are covered in various endpoints, this specific code path isn’t directly exercised by any test, as noted by codecov.
- Action Point: Please add a dedicated unit test for this error-wrapping path to explicitly validate its behavior or provide documentation explaining how its coverage is achieved indirectly via integration tests.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 79-79: internal/connectors/plugins/public/adyen/client/client.go#L79
Added line #L79 was not covered by testsinternal/connectors/plugins/public/mangopay/client/transactions.go (2)
11-11: Good update to use internal error utils package.Using the local error utilities package is a good practice for better maintenance and consistency.
54-57: Well-structured error handling improvement.The change from the previous error handling to
errorsutils.NewWrappedErrorimproves clarity by correctly separating the contextual message from the original error. Including the status code in the error message provides better debugging context.internal/connectors/plugins/public/moneycorp/client/transactions.go (3)
14-14: Good addition of error utilities import.Using a dedicated error utilities package helps standardize error handling across the codebase.
100-100: Type change frommoneycorpErrortomoneycorpErrors.The change from singular to plural form suggests a structural change in error handling to potentially support multiple errors. This aligns with the PR objective to improve connector error handling.
103-106: Improved error wrapping structure.The change implements a more structured approach to error handling by separating the contextual information from the original error. This makes it easier to understand the error origin and what happened during API communication.
internal/connectors/plugins/public/mangopay/client/refund.go (2)
9-9: Good update to use internal error utils package.Using the local error utilities package is a good practice for better maintenance and consistency.
45-48: Improved error wrapping structure.The updated error handling provides better context by including the status code in the error message while preserving the original error through wrapping. This aligns with the PR objective to improve error handling and messaging for connectors.
internal/connectors/plugins/public/bankingcircle/client/auth.go (2)
11-11: Good addition of error utilities import.Using a dedicated error utilities package helps standardize error handling across the codebase.
40-43:❓ Verification inconclusive
Improved error handling with detailed context.
The change enhances error reporting by including both the status code and the specific error text from the response in the wrapped error message, which provides better debugging context.
🏁 Script executed:
#!/bin/bash # Verify test coverage for the updated error handling # Find tests related to the bankingcircle client echo "Checking for tests related to the bankingcircle client:" fd "client_test.go" --exec grep -l "Test.*login" {} \; --no-ignore # Check if there are any tests that cover the error path echo "Checking for tests covering login error scenarios:" fd "client_test.go" --exec grep -l "failed to login" {} \; --no-ignoreLength of output: 375
Attention: Verify Improved Error Handling & Test Coverage
The updated error handling in
internal/connectors/plugins/public/bankingcircle/client/auth.go(lines 40–43) now reports both the HTTP status code and the specific error text, which is a clear improvement for debugging. However, the automated search for tests covering this error path (i.e. looking for the “failed to login” string) did not return any results. Please manually verify that there are appropriate tests exercising this error scenario. If such tests are missing, consider adding test cases that simulate login failures and confirm that the wrapped error message includes the expected details.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 40-43: internal/connectors/plugins/public/bankingcircle/client/auth.go#L40-L43
Added lines #L40 - L43 were not covered by testsinternal/connectors/plugins/public/wise/transfers.go (3)
11-11: Added import for error utilities.The new import of errorsutils from
github.com/formancehq/payments/internal/utils/errorsfacilitates the improved error handling implementation throughout this file.
24-27: Improved error handling for better client feedback.This change enhances error handling by properly wrapping the original error with
models.ErrInvalidRequest, which will likely result in returning a BadRequest response to the client rather than an internal server error. This approach provides clearer feedback when the issue is related to invalid user input.
32-35: Enhanced error handling for amount conversion.Similar to the previous error handling improvement, this change wraps the amount conversion error with
models.ErrInvalidRequest. This consistent approach to error handling helps distinguish between client errors and server errors, providing better feedback to API consumers.internal/connectors/plugins/public/mangopay/client/users.go (2)
10-10: Standardized error utilities import.Updated the import path for error utilities to use a local package rather than an external library, aligning with the standardization of error handling across the codebase.
36-39: Improved error handling with explicit status code.The new implementation uses
errorsutils.NewWrappedErrorto wrap the original error with a more descriptive message that includes the HTTP status code. This provides better context for debugging and troubleshooting API interactions.internal/connectors/plugins/public/moneycorp/client/transfers.go (5)
11-11: Added error utilities import.Added the import for errorsutils to support the improved error handling implementation in this file.
75-75: Updated error type from singular to plural form.Changed the type of
errResfrommoneycorpErrortomoneycorpErrors, suggesting a modification in how errors are structured or represented. This change should be consistent with updates to the error structure defined elsewhere.
78-81: Enhanced error handling for transfer initiation.Replaced direct error formatting with
errorsutils.NewWrappedError, which provides a more structured approach to wrapping errors. This ensures consistent error handling and maintains the original error context while adding descriptive information.
98-98: Updated error type from singular to plural form.Changed the type of
errResfrommoneycorpErrortomoneycorpErrors, consistent with the change in theInitiateTransfermethod.
101-104: Enhanced error handling for transfer retrieval.Improved error handling by using
errorsutils.NewWrappedErrorto wrap the original error, which provides better error classification and context for debugging.internal/connectors/plugins/public/modulr/client/transfer.go (5)
11-11: Added error utilities import.Added the import for errorsutils to support the improved error handling implementation in this file.
73-73: Updated error type from singular to plural form.Changed the type of
errResfrommodulrErrortomodulrErrors, suggesting a modification in how errors are structured or represented.
76-79: Enhanced error handling for transfer initiation.Replaced direct error formatting with
errorsutils.NewWrappedError, improving the error handling structure and consistency. This change ensures that error information is properly preserved while adding contextual information.
93-93: Updated error type from singular to plural form.Changed the type of
errResfrommodulrErrortomodulrErrors, consistent with the change in theInitiateTransfermethod.
96-99: Enhanced error handling for transfer retrieval.Improved error handling implementation by using
errorsutils.NewWrappedErrorto wrap the original error with additional context, making debugging and troubleshooting easier.internal/connectors/plugins/public/mangopay/payouts.go (3)
14-14: Good addition of error utilities package.Adding the error utilities package will allow for more consistent error handling throughout the codebase.
25-28: Improved error handling in validatePayoutRequest.The switch from simple
fmt.Errorftoerrorsutils.NewWrappedErrorenhances error handling by:
- Properly wrapping the error with
models.ErrInvalidRequestas context- Providing more structured errors to API consumers
- Making it easier to identify that these are validation errors
This aligns with the PR's objective of improving connector error handling.
Also applies to: 32-35, 39-42, 47-50, 54-57
72-75: Better error context in createPayout.The error message is now more descriptive by including the specific failure reason ("failed to get currency and precision from asset") while properly categorizing it as an invalid request error. This will help API consumers understand why their request failed.
internal/connectors/plugins/public/mangopay/bank_account_creation.go (2)
12-12: Good addition of error utilities package.Adding the error utilities package will allow for more consistent error handling throughout the codebase.
53-56: Greatly improved error handling across bank account creation.The refactoring of error handling in all bank account creation paths (IBAN, US, CA, GB, and other) follows good practices:
- Each error is now wrapped with
models.ErrFailedAccountCreationfor consistent categorization- The specific details about what failed (e.g., "failed to create IBAN bank account") are preserved
- The original error is properly wrapped and maintained in the chain
This implementation addresses the PR objective of improving connector error handling and sending appropriate error responses to API consumers instead of internal server errors.
Also applies to: 80-83, 103-106, 125-128, 153-156
internal/connectors/plugins/public/atlar/bank_account_creation.go (2)
8-8: Good addition of error utilities package.Adding the error utilities package will allow for more consistent error handling throughout the codebase.
14-17: Improved validation error handling.The switch from simple
fmt.Errorftoerrorsutils.NewWrappedErrorenhances error handling by:
- Properly wrapping errors with
models.ErrInvalidRequestfor consistent categorization- Maintaining detailed error messages that explain exactly what's wrong with the request
- Providing better error structure for API consumers
This change aligns with the PR objective of improving error handling and sending
BadRequestresponses instead of internal server errors.Also applies to: 22-25, 29-32
internal/connectors/plugins/public/mangopay/client/payout.go (3)
11-11: Updated import path for error utilities.The error utilities package has been moved from a shared library to an internal package, which provides better control over the error handling implementation.
65-71: Greatly improved error handling for InitiatePayout.The changes enhance error reporting by:
- Capturing detailed error information in the
errResvariable- Including the specific error message from the API response
- Properly wrapping the original error for context preservation
This provides API consumers with more actionable error information rather than generic internal errors.
89-92: Improved error context for GetPayout.The error now includes the HTTP status code in the error message, which provides valuable context for debugging and troubleshooting. This follows the consistent pattern of using
errorsutils.NewWrappedErrorthroughout the codebase.internal/connectors/plugins/public/modulr/client/payout.go (5)
11-11: Import for error handling utilities added.The addition of the error utilities import enhances error handling capabilities throughout the file.
50-50: Type change frommodulrErrortomodulrErrors.Changed variable type to use the plural form, which likely aligns with a structure change in the error handling system. The change appears consistent with the overall error handling improvements in this PR.
53-56: Improved error handling with structured wrapping.Replaced direct error formatting with
errorsutils.NewWrappedError, which provides better structure and context to the error. This change:
- Maintains the original error context
- Adds a formatted message with more details
- Aligns with the PR objective of improving connector error handling
70-70: Type change frommodulrErrortomodulrErrors.This change mirrors the earlier type update in the
InitiatePayoutmethod, ensuring consistency across the codebase.
73-76: Consistent error handling improvement.Applied the same structured error wrapping pattern to maintain consistency with the changes made in the
InitiatePayoutmethod.internal/connectors/plugins/public/atlar/bank_account_creation_test.go (3)
64-64: Updated test expectation with enhanced error message.The error message now includes ": invalid request" to provide clearer context that the error is related to validation of the request. This improvement aligns with the PR's goal of enhancing error messaging.
77-77: Consistent error message improvement.The error message for missing owner type field now follows the same pattern, adding ": invalid request" to indicate the error is related to request validation, improving diagnostic clarity.
90-90: Enhanced error message for incorrect field value.The error message for wrong owner type now includes ": invalid request" suffix, maintaining consistency with other validation errors while providing clearer context about the nature of the failure.
internal/connectors/plugins/public/moneycorp/transfers_test.go (3)
72-72: More contextual error message in test expectation.The error message now specifies "in transfer/payout request" to provide clearer context about where the validation error occurred. This improves error diagnostics for developers.
85-85: Consistent error message improvement.Modified the error message for missing destination account to follow the same pattern, adding "in transfer/payout request" for clarity and consistency with other validation error messages.
98-98: Enhanced error message with specific asset information.The updated error message now includes the specific problematic asset (HUF) that caused the validation error. This provides more precise debugging information when currency-related errors occur.
internal/connectors/plugins/public/bankingcircle/payouts.go (9)
11-11: Import for error handling utilities added.Added import for the error utilities package which is used throughout the file to implement the new error wrapping approach.
16-19: Structured error wrapping for source account validation.Replaced direct error formatting with
errorsutils.NewWrappedError, which:
- Provides a more descriptive error message
- Wraps the error with
models.ErrInvalidRequestto properly categorize it as a validation error- Aligns with the PR objective of sending BadRequest instead of internal server errors for user input issues
23-26: Consistent error wrapping for destination account validation.Applied the same structured error wrapping pattern for destination account validation, maintaining consistency and improving error clarity.
30-33: Structured error for destination account name validation.Continued the consistent use of
errorsutils.NewWrappedErrorfor the destination account name validation error, ensuring all validation errors are properly categorized.
38-41: Enhanced error handling for account identifiers validation.Implemented the structured error wrapping for the validation of required account number or IBAN metadata, maintaining a consistent approach to error handling throughout the validation process.
54-57: Improved error handling for currency and precision extraction.Applied the structured error wrapping approach to the currency and precision extraction error, providing more context and properly categorizing it as an invalid request error.
71-74: Structured error handling for source account retrieval.Improved error reporting for source account retrieval failures by implementing the structured error wrapping approach, providing more specific context about the failure.
77-80: Enhanced error for missing account identifiers.Implemented structured error wrapping for the case when no account identifiers are provided for the source account, improving error clarity.
62-65:❓ Verification inconclusive
Error wrapping for amount conversion with missing test coverage.
Implemented structured error wrapping for amount conversion errors, but static analysis indicates this section lacks test coverage.
Consider adding unit tests to cover the amount conversion error handling path. The static analysis tool indicates that lines 62-65 are not covered by tests, which is important for ensuring the error handling behaves as expected.
🏁 Script executed:
#!/bin/bash # Check if there are any tests for the createPayout function's amount conversion error path rg -i "Test.*createPayout.*amount.*error" --type goLength of output: 53
Test Coverage Required: Amount Conversion Error Handling
- The error wrapping implemented in
internal/connectors/plugins/public/bankingcircle/payouts.go(lines 62–65) appears to lack unit tests.- Static analysis (and our grep search) did not reveal tests exercising the failure path of the amount conversion in the
createPayoutfunction.- Please add unit tests that simulate a failure in obtaining the string amount from a big integer, and verify that the error is wrapped as expected with the proper error message.
Please manually verify that no duplicate tests exist and that this error path is fully covered.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 62-65: internal/connectors/plugins/public/bankingcircle/payouts.go#L62-L65
Added lines #L62 - L65 were not covered by testsinternal/connectors/engine/engine.go (5)
18-18: Good addition of the error utilities package.Adding the dedicated error utilities package is a good step toward standardizing error handling throughout the codebase.
120-120: Improved error wrapping pattern.Changed from
errors.Wrap(ErrValidation, err.Error())toerrorsutils.NewWrappedError(err, ErrValidation). This improves error handling by preserving the original error as the cause while settingErrValidationas the wrapper error.
138-138: Consistent error wrapping approach.Using the same error wrapping pattern as in the previous instance for validation errors. This maintains consistency across the codebase.
685-685: Consistent workflow error handling.Same improvement as in the CreateTransfer method, applying consistent error handling to CreatePayout as well.
This code path also isn't covered by tests. Consider adding test coverage for this error handling scenario.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 685-685: internal/connectors/engine/engine.go#L685
Added line #L685 was not covered by tests
567-567:❓ Verification inconclusive
Better error handling for workflow errors.
Using
handleWorkflowErrorprovides centralized error handling for workflow execution errors. This allows for consistent processing of error types, particularly for validation errors that should be returned as BadRequest instead of internal server errors.This code path isn't covered by tests according to codecov. Consider adding tests to verify this error handling path:
🏁 Script executed:
#!/bin/bash # Look for the handleWorkflowError implementation to understand what it does fd --type f "errors.go" --exec grep -l "handleWorkflowError" {} \;Length of output: 106
Error Handling Verified – Please Add Test Coverage
- Verified that
handleWorkflowErroris implemented in./internal/connectors/engine/errors.goand is used inengine.go(line 567) to centralize workflow error handling.- This centralized approach reliably ensures that validation errors trigger a BadRequest response instead of an internal server error.
- However, as noted by codecov, this code path remains untested. Please add tests that simulate errors from workflow execution to validate that
handleWorkflowErrorcorrectly transforms these errors into the appropriate HTTP responses.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 567-567: internal/connectors/engine/engine.go#L567
Added line #L567 was not covered by testsinternal/connectors/plugins/public/mangopay/bank_account_creation_test.go (5)
105-105: More descriptive error message.The error message now specifically mentions "IBAN bank account" rather than just "account", providing clearer context about what failed. This change improves troubleshooting and user experience.
182-182: Consistent error message improvement for US bank accounts.Similar to the IBAN case, the error message now specifies "US bank account" instead of just "account", making errors more descriptive and specific.
264-264: Improved error specificity for CA bank accounts.The error message now includes "CA bank account" to provide better context about the failure.
345-345: Enhanced error context for GB bank accounts.Another instance of improving error message specificity by mentioning "GB bank account".
425-425: Consistent error message improvement for other bank accounts.Final instance of the same pattern of improvement, explicitly mentioning "other bank account" in the error message.
internal/connectors/plugins/public/bankingcircle/client/payments.go (3)
11-11: Added error utilities import.Good addition for consistent error handling across the codebase.
147-151: Consistent error handling for individual payment retrieval.Similar improvement to the GetPayments method, providing better error structure and context.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 147-150: internal/connectors/plugins/public/bankingcircle/client/payments.go#L147-L150
Added lines #L147 - L150 were not covered by tests
174-178: Consistent error structure for payment status retrieval.Completes the pattern of improving error handling across all payment-related methods in this client.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 174-177: internal/connectors/plugins/public/bankingcircle/client/payments.go#L174-L177
Added lines #L174 - L177 were not covered by testsinternal/connectors/plugins/public/moneycorp/payouts.go (3)
13-13: Added error utilities package.Good addition for consistent error handling approach across plugins.
23-27: Improved error structure for currency/precision handling.Changed from concatenating errors with
%w: %wto using the structuredNewWrappedErrorapproach. This makes the error chain clearer by explicitly defining the cause and wrapper relationship.
31-35: More descriptive error for amount conversion failures.The error message now includes the actual amount value that failed conversion, making debugging easier. Also uses the same improved error structure as the previous change.
internal/connectors/plugins/public/mangopay/client/bank_accounts.go (3)
12-12: Import path updated for error utilities packageThe import path has been updated to use the local error utilities package instead of an external one, which helps standardize error handling across the codebase.
119-126: Improved error handling with structured error responseThe error handling has been enhanced to capture structured error information from the Mangopay API using the
errResvariable of typemangopayError. This is a good improvement as it:
- Captures specific error details from the API response
- Provides more context about what went wrong from the API's perspective
- Uses the standardized
errorsutils.NewWrappedErrorfunction for consistent error handlingThis change aligns well with the PR's objective of improving connector error handling.
154-158: Standardized error handling for HTTP errorsThe error handling for HTTP response errors has been standardized to use
errorsutils.NewWrappedError, with improved formatting that includes the status code. This provides better context about what went wrong during the API call and follows a consistent error handling pattern.internal/connectors/plugins/public/wise/payouts.go (4)
13-13: Added import for error utilities packageThe error utilities package has been imported to support the improved error handling throughout the file.
26-30: Enhanced error handling for currency validationThe error handling has been improved to use
errorsutils.NewWrappedErrorinstead of directly chaining errors withfmt.Errorf. This approach:
- Separates the detailed error message from the error type
- Makes it clearer that this is an invalid request error
- Facilitates returning a
BadRequestresponse instead of an internal server errorThis change aligns with the PR objective of enhancing error reporting for better client feedback.
34-38: Improved error details for amount conversion failuresThe error message now includes the actual amount value that failed conversion, providing more context for debugging. This, combined with the standardized error wrapping approach, makes errors more informative while maintaining consistency in error handling across the codebase.
66-70: Standardized error handling for unsupported currencyApplied the same error handling pattern for unsupported currencies, wrapping a specific error message with
models.ErrInvalidRequest. This ensures consistent error handling throughout the payment processing flow and improves the client experience by providing more specific error information.
| return errorsutils.NewWrappedError( | ||
| err, | ||
| httpwrapper.ErrStatusCodeTooManyRequests, | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Inconsistent argument order in NewWrappedError
The order of arguments in errorsutils.NewWrappedError here differs from other files in this PR. In other files, the new error message is the first argument and the original error is the second, but here it's reversed.
🏁 Script executed:
#!/bin/bash
# Check the NewWrappedError function definition to confirm expected argument order
echo "Checking NewWrappedError function definition..."
rg -A 5 "func NewWrappedError" internal/utils/errors
# Look for other usages of NewWrappedError in the codebase
echo "Looking for other examples of NewWrappedError usage..."
rg -l "NewWrappedError" --glob "*.go" | grep -v client.go | head -n 5 | xargs rg -A 3 "NewWrappedError\("Length of output: 6522
Action Required: Correct Argument Order in NewWrappedError Usage
It appears that the usage of errorsutils.NewWrappedError in internal/connectors/plugins/public/stripe/client/client.go (lines 75–78) is inconsistent with its application elsewhere in the codebase. In other files (e.g., those in the Wise plugin), callers first pass a formatted error via fmt.Errorf("…%w", err)—providing context—and then a constant error as the second argument. In this case, the original error (err) is being passed as the first argument instead of a formatted message. This inconsistency can lead to unexpected error wrapping and reduced clarity.
Suggested Fix:
- Update the call in
client.goso that it first constructs a formatted error message that wraps the original error, followed by the static error. For example, change the block to:return errorsutils.NewWrappedError( fmt.Errorf("stripe client error: %w", err), httpwrapper.ErrStatusCodeTooManyRequests, )
Please review and update the code accordingly to ensure consistency across the codebase.
internal/connectors/plugins/public/stripe/reverse_transfers_test.go
Outdated
Show resolved
Hide resolved
paul-nicolas
force-pushed
the
feat/improve-errors
branch
from
77b52eb to
84981e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
internal/connectors/engine/engine.go (1)
310-310:⚠️ Potential issueInconsistent error handling approach.
While most of the file has been updated to use
errorsutils.NewWrappedError, this line still uses the olderrors.Wrapapproach.Apply this diff to maintain consistency:
- return errors.Wrap(ErrValidation, err.Error()) + return errorsutils.NewWrappedError(err, ErrValidation)
🧹 Nitpick comments (2)
internal/connectors/plugins/public/bankingcircle/payouts.go (1)
110-122: Consider consistent error handling for remaining error cases.While most errors are now wrapped with
models.ErrInvalidRequest, the errors at lines 111, 116, and 121 are still returned directly. Consider whether these should also use the new error wrapping pattern for consistency, or if they represent true internal errors rather than client errors.if err != nil { - return nil, err + return nil, errorsutils.NewWrappedError( + fmt.Errorf("failed to initiate transfer or payout: %w", err), + models.ErrInternalServerError, // or models.ErrInvalidRequest if appropriate + ) }internal/connectors/plugins/public/adyen/client/client.go (1)
84-86: Consider consistent error wrapping for server errors as wellWhile you've updated the error handling for rate limiting and client errors to use
errorsutils.NewWrappedError, the server error case (500+) still usesfmt.Errorf. For consistency, consider updating this case as well.if statusCode >= http.StatusInternalServerError { - return fmt.Errorf("unexpected status code %d: %w", statusCode, err) + return errorsutils.NewWrappedError( + fmt.Errorf("unexpected status code %d", statusCode), + err, + ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (108)
internal/api/services/errors.go(2 hunks)internal/api/v2/errors.go(2 hunks)internal/connectors/engine/activities/errors.go(4 hunks)internal/connectors/engine/engine.go(6 hunks)internal/connectors/engine/errors.go(1 hunks)internal/connectors/engine/workflow/create_payout.go(3 hunks)internal/connectors/engine/workflow/create_transfer.go(3 hunks)internal/connectors/engine/workflow/update_tasks.go(2 hunks)internal/connectors/plugins/currency/currency.go(2 hunks)internal/connectors/plugins/public/adyen/client/client.go(2 hunks)internal/connectors/plugins/public/atlar/bank_account_creation.go(2 hunks)internal/connectors/plugins/public/atlar/bank_account_creation_test.go(3 hunks)internal/connectors/plugins/public/atlar/client/client.go(2 hunks)internal/connectors/plugins/public/atlar/client/counter_parties.go(2 hunks)internal/connectors/plugins/public/atlar/payouts.go(2 hunks)internal/connectors/plugins/public/atlar/payouts_test.go(3 hunks)internal/connectors/plugins/public/atlar/utils.go(2 hunks)internal/connectors/plugins/public/bankingcircle/client/accounts.go(3 hunks)internal/connectors/plugins/public/bankingcircle/client/auth.go(2 hunks)internal/connectors/plugins/public/bankingcircle/client/client.go(2 hunks)internal/connectors/plugins/public/bankingcircle/client/payments.go(4 hunks)internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go(2 hunks)internal/connectors/plugins/public/bankingcircle/payouts.go(2 hunks)internal/connectors/plugins/public/bankingcircle/payouts_test.go(7 hunks)internal/connectors/plugins/public/bankingcircle/transfers.go(2 hunks)internal/connectors/plugins/public/bankingcircle/transfers_test.go(7 hunks)internal/connectors/plugins/public/currencycloud/client/accounts.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/auth.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/balances.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/beneficiaries.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/contacts.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/payouts.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/transactions.go(2 hunks)internal/connectors/plugins/public/currencycloud/client/transfers.go(2 hunks)internal/connectors/plugins/public/currencycloud/payouts.go(2 hunks)internal/connectors/plugins/public/currencycloud/payouts_test.go(3 hunks)internal/connectors/plugins/public/currencycloud/transfers.go(2 hunks)internal/connectors/plugins/public/currencycloud/transfers_test.go(3 hunks)internal/connectors/plugins/public/generic/balances.go(1 hunks)internal/connectors/plugins/public/mangopay/bank_account_creation.go(6 hunks)internal/connectors/plugins/public/mangopay/bank_account_creation_test.go(5 hunks)internal/connectors/plugins/public/mangopay/client/bank_accounts.go(3 hunks)internal/connectors/plugins/public/mangopay/client/payin.go(2 hunks)internal/connectors/plugins/public/mangopay/client/payout.go(3 hunks)internal/connectors/plugins/public/mangopay/client/refund.go(2 hunks)internal/connectors/plugins/public/mangopay/client/transactions.go(2 hunks)internal/connectors/plugins/public/mangopay/client/transfer.go(3 hunks)internal/connectors/plugins/public/mangopay/client/users.go(2 hunks)internal/connectors/plugins/public/mangopay/client/wallets.go(3 hunks)internal/connectors/plugins/public/mangopay/client/webhooks.go(4 hunks)internal/connectors/plugins/public/mangopay/payouts.go(3 hunks)internal/connectors/plugins/public/mangopay/payouts_test.go(6 hunks)internal/connectors/plugins/public/mangopay/plugin.go(3 hunks)internal/connectors/plugins/public/mangopay/transfers.go(2 hunks)internal/connectors/plugins/public/mangopay/transfers_test.go(5 hunks)internal/connectors/plugins/public/mangopay/webhooks.go(2 hunks)internal/connectors/plugins/public/mangopay/webhooks_test.go(3 hunks)internal/connectors/plugins/public/modulr/client/accounts.go(3 hunks)internal/connectors/plugins/public/modulr/client/beneficiaries.go(2 hunks)internal/connectors/plugins/public/modulr/client/error.go(2 hunks)internal/connectors/plugins/public/modulr/client/payments.go(2 hunks)internal/connectors/plugins/public/modulr/client/payout.go(3 hunks)internal/connectors/plugins/public/modulr/client/transactions.go(2 hunks)internal/connectors/plugins/public/modulr/client/transfer.go(3 hunks)internal/connectors/plugins/public/modulr/payouts.go(2 hunks)internal/connectors/plugins/public/modulr/payouts_test.go(3 hunks)internal/connectors/plugins/public/modulr/transfers.go(2 hunks)internal/connectors/plugins/public/modulr/transfers_test.go(3 hunks)internal/connectors/plugins/public/modulr/utils.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/accounts.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/auth.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/balances.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/error.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/payouts.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/recipients.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/transactions.go(2 hunks)internal/connectors/plugins/public/moneycorp/client/transfers.go(3 hunks)internal/connectors/plugins/public/moneycorp/payouts.go(2 hunks)internal/connectors/plugins/public/moneycorp/payouts_test.go(3 hunks)internal/connectors/plugins/public/moneycorp/transfers.go(2 hunks)internal/connectors/plugins/public/moneycorp/transfers_test.go(3 hunks)internal/connectors/plugins/public/moneycorp/utils.go(1 hunks)internal/connectors/plugins/public/stripe/client/client.go(2 hunks)internal/connectors/plugins/public/stripe/create_payouts.go(2 hunks)internal/connectors/plugins/public/stripe/create_payouts_test.go(2 hunks)internal/connectors/plugins/public/stripe/create_transfers.go(2 hunks)internal/connectors/plugins/public/stripe/create_transfers_test.go(2 hunks)internal/connectors/plugins/public/stripe/reverse_transfers.go(2 hunks)internal/connectors/plugins/public/stripe/reverse_transfers_test.go(1 hunks)internal/connectors/plugins/public/stripe/utils.go(1 hunks)internal/connectors/plugins/public/wise/client/balances.go(3 hunks)internal/connectors/plugins/public/wise/client/payouts.go(3 hunks)internal/connectors/plugins/public/wise/client/profiles.go(2 hunks)internal/connectors/plugins/public/wise/client/quotes.go(2 hunks)internal/connectors/plugins/public/wise/client/recipient_accounts.go(3 hunks)internal/connectors/plugins/public/wise/client/transfers.go(4 hunks)internal/connectors/plugins/public/wise/client/webhooks.go(4 hunks)internal/connectors/plugins/public/wise/config.go(2 hunks)internal/connectors/plugins/public/wise/payouts.go(3 hunks)internal/connectors/plugins/public/wise/payouts_test.go(7 hunks)internal/connectors/plugins/public/wise/transfers.go(2 hunks)internal/connectors/plugins/public/wise/transfers_test.go(7 hunks)internal/connectors/plugins/public/wise/utils.go(1 hunks)internal/connectors/plugins/registry/errors.go(2 hunks)internal/connectors/plugins/registry/plugins.go(1 hunks)internal/utils/errors/errors.go(1 hunks)internal/utils/errors/errors_test.go(1 hunks)test/e2e/api_connectors_test.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/e2e/api_connectors_test.go
🚧 Files skipped from review as they are similar to previous changes (85)
- internal/connectors/plugins/public/stripe/reverse_transfers_test.go
- internal/connectors/plugins/public/bankingcircle/client/client.go
- internal/connectors/plugins/public/mangopay/webhooks.go
- internal/connectors/plugins/public/stripe/client/client.go
- internal/connectors/plugins/public/atlar/payouts.go
- internal/connectors/plugins/public/modulr/payouts.go
- internal/connectors/plugins/currency/currency.go
- internal/connectors/engine/workflow/update_tasks.go
- internal/connectors/plugins/public/atlar/utils.go
- internal/connectors/plugins/public/currencycloud/client/payouts.go
- internal/connectors/plugins/public/currencycloud/client/transfers.go
- internal/connectors/plugins/public/stripe/reverse_transfers.go
- internal/connectors/plugins/public/stripe/create_payouts_test.go
- internal/connectors/plugins/public/atlar/payouts_test.go
- internal/connectors/plugins/public/moneycorp/client/auth.go
- internal/connectors/plugins/public/moneycorp/client/accounts.go
- internal/connectors/plugins/public/modulr/client/beneficiaries.go
- internal/connectors/engine/workflow/create_payout.go
- internal/connectors/plugins/public/moneycorp/transfers_test.go
- internal/connectors/plugins/public/stripe/create_transfers.go
- internal/connectors/engine/workflow/create_transfer.go
- internal/api/v2/errors.go
- internal/connectors/plugins/public/modulr/utils.go
- internal/api/services/errors.go
- internal/connectors/plugins/public/currencycloud/payouts_test.go
- internal/connectors/plugins/public/wise/utils.go
- internal/connectors/plugins/public/mangopay/plugin.go
- internal/connectors/plugins/public/wise/client/quotes.go
- internal/connectors/plugins/public/currencycloud/transfers_test.go
- internal/utils/errors/errors_test.go
- internal/connectors/plugins/public/moneycorp/payouts_test.go
- internal/connectors/plugins/public/modulr/transfers_test.go
- internal/connectors/plugins/public/wise/config.go
- internal/connectors/plugins/public/modulr/client/transactions.go
- internal/connectors/plugins/public/stripe/utils.go
- internal/connectors/plugins/public/mangopay/client/refund.go
- internal/connectors/plugins/public/mangopay/client/transactions.go
- internal/connectors/plugins/public/mangopay/transfers.go
- internal/connectors/plugins/public/moneycorp/client/recipients.go
- internal/connectors/plugins/registry/plugins.go
- internal/connectors/plugins/public/wise/client/transfers.go
- internal/connectors/plugins/public/moneycorp/transfers.go
- internal/connectors/plugins/public/modulr/client/payout.go
- internal/connectors/plugins/public/stripe/create_transfers_test.go
- internal/connectors/plugins/public/wise/transfers.go
- internal/connectors/plugins/public/wise/client/webhooks.go
- internal/connectors/plugins/public/mangopay/client/users.go
- internal/connectors/plugins/public/moneycorp/utils.go
- internal/connectors/plugins/public/mangopay/client/wallets.go
- internal/connectors/plugins/public/wise/client/recipient_accounts.go
- internal/connectors/plugins/public/moneycorp/client/error.go
- internal/connectors/plugins/public/stripe/create_payouts.go
- internal/connectors/plugins/public/mangopay/webhooks_test.go
- internal/connectors/plugins/public/wise/client/profiles.go
- internal/utils/errors/errors.go
- internal/connectors/plugins/public/moneycorp/client/transfers.go
- internal/connectors/plugins/public/currencycloud/transfers.go
- internal/connectors/plugins/public/mangopay/client/bank_accounts.go
- internal/connectors/plugins/public/modulr/payouts_test.go
- internal/connectors/plugins/public/wise/client/payouts.go
- internal/connectors/plugins/public/atlar/bank_account_creation_test.go
- internal/connectors/plugins/public/currencycloud/client/transactions.go
- internal/connectors/plugins/public/moneycorp/client/balances.go
- internal/connectors/plugins/public/wise/client/balances.go
- internal/connectors/plugins/public/mangopay/payouts_test.go
- internal/connectors/plugins/public/modulr/client/payments.go
- internal/connectors/plugins/public/wise/payouts.go
- internal/connectors/plugins/public/modulr/client/transfer.go
- internal/connectors/plugins/public/mangopay/payouts.go
- internal/connectors/plugins/public/mangopay/transfers_test.go
- internal/connectors/plugins/public/bankingcircle/payouts_test.go
- internal/connectors/plugins/public/bankingcircle/transfers_test.go
- internal/connectors/plugins/public/moneycorp/client/payouts.go
- internal/connectors/plugins/public/generic/balances.go
- internal/connectors/plugins/public/mangopay/client/payin.go
- internal/connectors/plugins/public/modulr/transfers.go
- internal/connectors/plugins/public/mangopay/bank_account_creation.go
- internal/connectors/plugins/public/mangopay/bank_account_creation_test.go
- internal/connectors/plugins/public/wise/payouts_test.go
- internal/connectors/plugins/registry/errors.go
- internal/connectors/plugins/public/currencycloud/payouts.go
- internal/connectors/plugins/public/mangopay/client/transfer.go
- internal/connectors/plugins/public/moneycorp/payouts.go
- internal/connectors/plugins/public/mangopay/client/webhooks.go
- internal/connectors/plugins/public/wise/transfers_test.go
🧰 Additional context used
🧬 Code Definitions (7)
internal/connectors/engine/errors.go (2)
internal/connectors/engine/engine.go (1)
New(91-106)internal/connectors/engine/plugins/plugin.go (2)
New(51-62)ErrValidation(23-23)
internal/connectors/engine/activities/errors.go (3)
internal/connectors/plugins/errors.go (3)
ErrNotImplemented(8-8)ErrInvalidClientRequest(10-10)ErrCurrencyNotSupported(12-12)internal/storage/error.go (1)
ErrNotFound(17-17)internal/models/errors.go (1)
ErrMissingConnectorMetadata(15-15)
internal/connectors/engine/engine.go (5)
internal/connectors/engine/plugins/plugin.go (1)
ErrValidation(23-23)internal/connectors/engine/errors.go (1)
ErrValidation(11-11)internal/api/services/errors.go (1)
ErrValidation(12-12)internal/api/v2/errors.go (1)
ErrValidation(19-19)internal/models/tasks.go (1)
Task(19-33)
internal/connectors/plugins/public/adyen/client/client.go (1)
internal/connectors/httpwrapper/client.go (2)
ErrStatusCodeTooManyRequests(21-21)ErrStatusCodeClientError(19-19)
internal/connectors/plugins/public/atlar/bank_account_creation.go (1)
internal/models/plugin.go (1)
CreateBankAccountResponse(116-118)
internal/connectors/plugins/public/atlar/client/client.go (1)
internal/connectors/httpwrapper/client.go (3)
ErrStatusCodeTooManyRequests(21-21)ErrStatusCodeClientError(19-19)ErrStatusCodeServerError(20-20)
internal/connectors/plugins/public/bankingcircle/payouts.go (4)
internal/models/plugin.go (1)
Plugin(14-36)internal/models/bank_accounts.go (2)
BankAccountAccountNumberMetadataKey(19-19)BankAccountIBANMetadataKey(20-20)internal/connectors/plugins/public/bankingcircle/client/payments.go (1)
Amount(59-62)internal/connectors/plugins/public/bankingcircle/client/accounts.go (1)
Account(29-40)
🪛 GitHub Check: codecov/patch
internal/connectors/engine/errors.go
[warning] 17-27: internal/connectors/engine/errors.go#L17-L27
Added lines #L17 - L27 were not covered by tests
[warning] 31-31: internal/connectors/engine/errors.go#L31
Added line #L31 was not covered by tests
internal/connectors/engine/activities/errors.go
[warning] 45-45: internal/connectors/engine/activities/errors.go#L45
Added line #L45 was not covered by tests
[warning] 61-61: internal/connectors/engine/activities/errors.go#L61
Added line #L61 was not covered by tests
[warning] 73-73: internal/connectors/engine/activities/errors.go#L73
Added line #L73 was not covered by tests
internal/connectors/engine/engine.go
[warning] 567-567: internal/connectors/engine/engine.go#L567
Added line #L567 was not covered by tests
[warning] 685-685: internal/connectors/engine/engine.go#L685
Added line #L685 was not covered by tests
internal/connectors/plugins/public/adyen/client/client.go
[warning] 75-75: internal/connectors/plugins/public/adyen/client/client.go#L75
Added line #L75 was not covered by tests
[warning] 79-79: internal/connectors/plugins/public/adyen/client/client.go#L79
Added line #L79 was not covered by tests
internal/connectors/plugins/public/atlar/bank_account_creation.go
[warning] 50-54: internal/connectors/plugins/public/atlar/bank_account_creation.go#L50-L54
Added lines #L50 - L54 were not covered by tests
internal/connectors/plugins/public/atlar/client/client.go
[warning] 104-104: internal/connectors/plugins/public/atlar/client/client.go#L104
Added line #L104 was not covered by tests
[warning] 108-108: internal/connectors/plugins/public/atlar/client/client.go#L108
Added line #L108 was not covered by tests
[warning] 110-110: internal/connectors/plugins/public/atlar/client/client.go#L110
Added line #L110 was not covered by tests
internal/connectors/plugins/public/atlar/client/counter_parties.go
[warning] 70-73: internal/connectors/plugins/public/atlar/client/counter_parties.go#L70-L73
Added lines #L70 - L73 were not covered by tests
internal/connectors/plugins/public/bankingcircle/client/accounts.go
[warning] 75-78: internal/connectors/plugins/public/bankingcircle/client/accounts.go#L75-L78
Added lines #L75 - L78 were not covered by tests
[warning] 98-101: internal/connectors/plugins/public/bankingcircle/client/accounts.go#L98-L101
Added lines #L98 - L101 were not covered by tests
internal/connectors/plugins/public/bankingcircle/client/auth.go
[warning] 40-43: internal/connectors/plugins/public/bankingcircle/client/auth.go#L40-L43
Added lines #L40 - L43 were not covered by tests
[warning] 45-48: internal/connectors/plugins/public/bankingcircle/client/auth.go#L45-L48
Added lines #L45 - L48 were not covered by tests
internal/connectors/plugins/public/bankingcircle/client/payments.go
[warning] 123-126: internal/connectors/plugins/public/bankingcircle/client/payments.go#L123-L126
Added lines #L123 - L126 were not covered by tests
[warning] 147-150: internal/connectors/plugins/public/bankingcircle/client/payments.go#L147-L150
Added lines #L147 - L150 were not covered by tests
[warning] 174-177: internal/connectors/plugins/public/bankingcircle/client/payments.go#L174-L177
Added lines #L174 - L177 were not covered by tests
internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go
[warning] 60-63: internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go#L60-L63
Added lines #L60 - L63 were not covered by tests
internal/connectors/plugins/public/bankingcircle/payouts.go
[warning] 62-65: internal/connectors/plugins/public/bankingcircle/payouts.go#L62-L65
Added lines #L62 - L65 were not covered by tests
internal/connectors/plugins/public/bankingcircle/transfers.go
[warning] 47-50: internal/connectors/plugins/public/bankingcircle/transfers.go#L47-L50
Added lines #L47 - L50 were not covered by tests
internal/connectors/plugins/public/currencycloud/client/accounts.go
[warning] 53-56: internal/connectors/plugins/public/currencycloud/client/accounts.go#L53-L56
Added lines #L53 - L56 were not covered by tests
internal/connectors/plugins/public/currencycloud/client/auth.go
[warning] 40-43: internal/connectors/plugins/public/currencycloud/client/auth.go#L40-L43
Added lines #L40 - L43 were not covered by tests
internal/connectors/plugins/public/currencycloud/client/balances.go
[warning] 57-60: internal/connectors/plugins/public/currencycloud/client/balances.go#L57-L60
Added lines #L57 - L60 were not covered by tests
internal/connectors/plugins/public/currencycloud/client/beneficiaries.go
[warning] 56-59: internal/connectors/plugins/public/currencycloud/client/beneficiaries.go#L56-L59
Added lines #L56 - L59 were not covered by tests
internal/connectors/plugins/public/currencycloud/client/contacts.go
[warning] 43-46: internal/connectors/plugins/public/currencycloud/client/contacts.go#L43-L46
Added lines #L43 - L46 were not covered by tests
[warning] 50-53: internal/connectors/plugins/public/currencycloud/client/contacts.go#L50-L53
Added lines #L50 - L53 were not covered by tests
🔇 Additional comments (71)
internal/connectors/plugins/public/modulr/client/accounts.go (5)
11-11: Good addition of the error utilities package.Adding the dedicated error utilities package aligns well with the PR objective to improve connector error handling.
50-50: Type change frommodulrErrortomodulrErrors.The change from singular to plural form suggests this now handles multiple error cases or provides a more comprehensive error structure, which aligns with the goal of improving error messaging.
53-57: Better error handling using structured approach.The implementation of
errorsutils.NewWrappedErrorimproves error handling by:
- Preserving the original error context
- Adding meaningful contextual information
- Providing a consistent error wrapping pattern
This change supports the PR objective of enhancing error handling and avoiding storing full stack traces.
70-70: Consistent type change tomodulrErrors.Good consistency with the previous change in the
GetAccountsmethod.
73-77: Consistent error handling approach.The error handling implementation mirrors the approach used in
GetAccounts, maintaining consistency throughout the codebase.internal/connectors/plugins/public/bankingcircle/payouts.go (9)
11-11: Import added for error utilities.The
errorsutilspackage is now imported to support the new error wrapping pattern implemented throughout this file.
16-19: Improved error handling for source account validation.Replacing direct error returns with
errorsutils.NewWrappedErrorthat wraps the error withmodels.ErrInvalidRequestwill properly categorize this as a client error rather than an internal server error.
23-26: Improved error handling for destination account validation.This follows the same pattern as other validation checks, ensuring consistent error handling by wrapping the error with
models.ErrInvalidRequest.
30-33: Improved error handling for destination account name validation.Consistent implementation of the new error handling pattern for destination account name validation.
38-41: Improved error handling for account number/IBAN validation.The error is properly wrapped with
models.ErrInvalidRequest, maintaining consistency with the other validation checks.
54-57: Improved error handling for currency and precision extraction.Error from
currency.GetCurrencyAndPrecisionFromAssetis now wrapped withmodels.ErrInvalidRequest, providing clearer context about the failure source while ensuring it's properly categorized as a client error.
71-74: Improved error handling for source account retrieval.The error from
p.client.GetAccountis now wrapped withmodels.ErrInvalidRequest, providing better context while ensuring proper error categorization.
77-80: Improved error handling for source account identifiers validation.Error for missing account identifiers is now wrapped with
models.ErrInvalidRequest, consistent with the new error handling pattern.
62-65:✅ Verification successful
Improved error handling for amount conversion.
Error from amount conversion is now properly wrapped with
models.ErrInvalidRequest. This change aligns with the PR objective to improve error handling for connectors.
🏁 Script executed:
#!/bin/bash # Check test coverage for this section that was flagged by CodeCov echo "Checking test coverage for amount conversion error handling..." rg -A 5 -B 5 "GetStringAmountFromBigIntWithPrecision" --glob "*_test.go"Length of output: 1858
Error handling for amount conversion verified.
The updated code in
internal/connectors/plugins/public/bankingcircle/payouts.go(lines 62–65) correctly wraps errors from the amount conversion withmodels.ErrInvalidRequest. The test coverage ininternal/connectors/plugins/currency/amount_test.goconfirms that the changes work as intended. No further modifications are required.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 62-65: internal/connectors/plugins/public/bankingcircle/payouts.go#L62-L65
Added lines #L62 - L65 were not covered by testsinternal/connectors/plugins/public/bankingcircle/client/accounts.go (2)
11-11: Good addition of the error utilities import.The import of the error utilities package aligns with the PR objective to improve connector error handling.
98-101:❓ Verification inconclusive
Consistent error handling implementation.
The error handling approach is consistent with the changes in the
GetAccountsmethod, maintaining a unified error handling strategy across the file. This consistency is important for maintainability.Similar to the other change, this new code lacks test coverage according to static analysis.
🏁 Script executed:
#!/bin/bash # Check if there are any tests for the GetAccount function grep -r "func.*Test.*GetAccount" --include="*.go" .Length of output: 53
Action Required: Verify Test Coverage for GetAccount Error Handling
The error handling implemented (lines 98–101 in
internal/connectors/plugins/public/bankingcircle/client/accounts.go) is consistent with changes applied in theGetAccountsmethod, which promotes maintainability. However, our static analysis (via grep for test functions) did not reveal any tests specifically covering theGetAccountfunctionality.
- File:
internal/connectors/plugins/public/bankingcircle/client/accounts.go(lines 98–101)- Observation: Error wrapping and reporting are consistent with similar methods.
- Concern: There is no evidence of test coverage for
GetAccount. Please verify manually or add appropriate tests to ensure robustness.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 98-101: internal/connectors/plugins/public/bankingcircle/client/accounts.go#L98-L101
Added lines #L98 - L101 were not covered by testsinternal/connectors/plugins/public/atlar/client/client.go (2)
12-12: Good import addition for improved error handling.Adding the
errorsutilspackage import is appropriate for the standardized error handling approach being implemented throughout the codebase.
104-104:❓ Verification inconclusive
Improved error wrapping approach with standardized utility.
The switch from
fmt.Errorftoerrorsutils.NewWrappedErrorprovides a more consistent and structured approach to error handling, which aligns with the PR's goal of improving connector error management. This change likely helps prevent storing full stack traces in the database and enables better error classification.However, these changes lack test coverage according to the code coverage report.
Consider adding tests to ensure the error wrapping behavior works as expected:
Also applies to: 108-108, 110-110
🏁 Script executed:
#!/bin/bash # Check for existing tests that might verify this error handling behavior rg -l "TestWrapSDKErr|test.*wrapSDKErr" --type go internal/connectors/plugins/public/atlar # See if there are any examples of testing similar error wrapping functions rg -l "Test.*NewWrappedError" --type goLength of output: 134
Test Coverage Needed for New Error Wrapping Approach
The switch to using
errorsutils.NewWrappedErrorclearly improves error handling consistency, but our searches didn’t return any test files validating this behavior for the affected lines (104, 108, and 110 ininternal/connectors/plugins/public/atlar/client/client.go). This absence suggests that the new error wrapping logic isn’t covered by tests, which is essential for ensuring that errors are properly wrapped without leaking full stack traces.
- Action Required: Please add tests that verify:
- The error wrapping correctly encapsulates the original error.
- No unintended full stack traces are stored (consistent with our error classification goals).
- Similar behavior across the affected lines (104, 108, 110).
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 104-104: internal/connectors/plugins/public/atlar/client/client.go#L104
Added line #L104 was not covered by testsinternal/connectors/plugins/public/currencycloud/client/accounts.go (1)
10-10: Import added for new error handling utility.This import supports the new standardized error handling approach being implemented across the codebase.
internal/connectors/plugins/public/bankingcircle/client/auth.go (2)
11-11: Appropriate import addition.Adding the error utilities import is consistent with the PR objective to improve connector error handling.
45-49:❓ Verification inconclusive
Improved error handling for general login failures.
This change improves error handling by wrapping the error with the status code context. It follows the same pattern as the previous error case, ensuring consistent error handling throughout the login process.
As noted by static analysis, consider adding test coverage for this error scenario as well.
🌐 Web query:
What is the best practice for wrapping HTTP client errors in Go?💡 Result:
The best practice for wrapping HTTP client errors in Go involves leveraging error wrapping to preserve context, using custom error types for specific handling, and centralizing error processing. Here's a structured approach:
Proper Error Wrapping
- Use fmt.Errorf with %w verb to wrap errors while preserving the original error chain:
if _, err := client.Do(req); err != nil { return fmt.Errorf("API request failed: %w", err) }- Leverage Go 1.13+ error inspection with
errors.Isanderrors.Asto check wrapped errors[3][4]Custom Error Types
Implement error types with HTTP metadata:
type HTTPError struct { StatusCode int Err error } func (e *HTTPError) Error() string { return fmt.Sprintf("HTTP %d: %v", e.StatusCode, e.Err) } func (e *HTTPError) Unwrap() error { return e.Err }Centralized Error Handling
Create handler middleware to convert errors to HTTP responses:
func ErrorHandler(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { err := next.ServeHTTP(w, r) if err != nil { var httpErr *HTTPError if errors.As(err, &httpErr) { w.WriteHeader(httpErr.StatusCode) json.NewEncoder(w).Encode(map[string]string{ "error": httpErr.Error(), }) } else { w.WriteHeader(500) } } }) }Key practices for HTTP clients:
- Wrap Context Errors: Explicitly handle context-related errors:
if errors.Is(err, context.DeadlineExceeded) { return &HTTPError{408, err} }- Network Error Detection: Use type assertions for network issues:
var dnsErr *net.DNSError if errors.As(err, &dnsErr) { // Handle DNS failure }- Status Code Mapping: Map common error types to HTTP status codes:
switch { case errors.Is(err, sql.ErrNoRows): return &HTTPError{404, err} case os.IsTimeout(err): return &HTTPError{504, err} }Error Context Preservation
- Maintain original errors for logging/debugging while returning sanitized versions to clients[4][10]
- Use sentinel errors for API boundary definitions:
var ErrInvalidRequest = errors.New("invalid request") // Usage return fmt.Errorf("%w: bad parameter %q", ErrInvalidRequest, param)For HTTP servers, combine this with handler adapters that automatically convert errors to HTTP responses[5][8]. This approach ensures proper error context while maintaining secure API boundaries and enabling centralized logging.
Citations:
- 1: https://www.joeshaw.org/error-handling-in-go-http-applications/
- 2: https://haisum.github.io/2021/08/14/2021-golang-http-errors/
- 3: https://bitfieldconsulting.com/posts/wrapping-errors
- 4: https://earthly.dev/blog/golang-errors/
- 5: https://blog.questionable.services/article/http-handler-error-handling-revisited/
- 6: net/http: Client does not wrap context errors golang/go#50856
- 7: https://blog.logrocket.com/error-handling-golang-best-practices/
- 8: https://markphelps.me/posts/handling-errors-in-your-http-handlers/
- 9: https://rollbar.com/blog/golang-wrap-and-unwrap-error/
- 10: https://boldlygo.tech/posts/2024-01-08-error-handling/
- 11: https://www.jetbrains.com/guide/go/tutorials/handle_errors_in_go/best_practices/
- 12: https://pkg.go.dev/net/http
- 13: https://www.youtube.com/watch?v=aS1cJfQ-LrQ
Attention: Verify Error Wrapping Implementation & Add Test Coverage
The updated code now wraps login failure errors by including the status code context, aligning with our previous patterns. Based on best practices for HTTP client error wrapping in Go—which recommend using
%wwithfmt.Errorfto support error unwrapping—please ensure that the custom functionerrorsutils.NewWrappedErrorinternally preserves the original error chain (e.g., via%wor an equivalent mechanism). Additionally, as noted by static analysis, please add test coverage for this error scenario to ensure robust error handling.
- Verify that
errorsutils.NewWrappedErrorexposes the wrapped error (usingUnwrap(), if applicable) to allow error inspection througherrors.Is/errors.As.- Add tests that simulate a login failure to confirm the error is correctly wrapped and propagated.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 45-48: internal/connectors/plugins/public/bankingcircle/client/auth.go#L45-L48
Added lines #L45 - L48 were not covered by testsinternal/connectors/plugins/public/currencycloud/client/balances.go (2)
11-11: Good addition of error utilities import.Adding the dedicated error utilities package is a good step towards standardizing error handling across the codebase.
57-61:❓ Verification inconclusive
Improved error handling with structured wrapping.
The change from a simple
fmt.Errorftoerrorsutils.NewWrappedErrorprovides better error context by separating the error message from the original error. This allows for more precise error handling and debugging.
🏁 Script executed:
#!/bin/bash # Check test coverage for the modified error handling echo "Checking if error handling is tested in balances.go:" rg -A 3 -B 3 "TestGetBalances.*err" --type goLength of output: 162
Review: Confirm Test Coverage for Enhanced Error Handling
The switch to using
errorsutils.NewWrappedErrorininternal/connectors/plugins/public/currencycloud/client/balances.goindeed improves error context. However, our search for tests (e.g., functions likeTestGetBalances) did not return any evidence that this change is being verified via automated tests. Please manually verify or add appropriate test cases to ensure that the new error wrapping behavior is properly covered.
- File:
internal/connectors/plugins/public/currencycloud/client/balances.go(Lines 57-61)- Recommendation: Validate by manually checking for tests in related test files or add tests that specifically confirm the structured error wrapping.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 57-60: internal/connectors/plugins/public/currencycloud/client/balances.go#L57-L60
Added lines #L57 - L60 were not covered by testsinternal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go (2)
12-12: Good addition of error utilities import.Adding the dedicated error utilities package is a good step towards standardizing error handling across the codebase.
60-64:❓ Verification inconclusive
Improved error structure with wrapped errors.
The change to
errorsutils.NewWrappedErrorcreates a more structured error that maintains the HTTP status code context while properly wrapping the original error. This provides better error handling capabilities downstream.
🏁 Script executed:
#!/bin/bash # Check test coverage for the modified error handling echo "Checking if error handling is tested in transfer_payouts.go:" rg -A 3 -B 3 "TestInitiateTransferOrPayouts.*err" --type goLength of output: 192
Action Required: Verify test coverage for improved error handling
The updated use of
errorsutils.NewWrappedErrorcorrectly encapsulates the HTTP status code and the original error for better downstream error handling. However, the automated search for tests specific to this error path did not yield any output. Please manually verify that there are tests covering this behavior or consider adding a test case that validates this error handling.
- Location:
internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go(Lines 60-64)- Code snippet:
return nil, errorsutils.NewWrappedError( fmt.Errorf("failed to make payment, status code %d", statusCode), err, )🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 60-63: internal/connectors/plugins/public/bankingcircle/client/transfer_payouts.go#L60-L63
Added lines #L60 - L63 were not covered by testsinternal/connectors/plugins/public/atlar/client/counter_parties.go (2)
10-10: Good addition of error utilities import.Adding the dedicated error utilities package is a good step towards standardizing error handling across the codebase.
70-74:❓ Verification inconclusive
Improved error structure with better separation of concerns.
The implementation of
errorsutils.NewWrappedErrorseparates the descriptive error message from the wrappedhttpwrapper.ErrStatusCodeUnexpectederror, which is cleaner and provides better context when errors are handled downstream.
🏁 Script executed:
#!/bin/bash # Check test coverage for the modified error handling echo "Checking if error handling is tested in counter_parties.go:" rg -A 3 -B 3 "TestPostV1CounterParties.*err" --type goLength of output: 185
Attention: Confirm Test Coverage for Enhanced Error Handling
The improved error structure using
errorsutils.NewWrappedErrorin
internal/connectors/plugins/public/atlar/client/counter_parties.go(lines 70–74) cleanly separates the descriptive error message from the HTTP error code. However, our initial search for corresponding test coverage did not yield any results. Please verify manually that tests exist (or add new ones) to cover this error scenario and ensure robust downstream error handling.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 70-73: internal/connectors/plugins/public/atlar/client/counter_parties.go#L70-L73
Added lines #L70 - L73 were not covered by testsinternal/connectors/engine/errors.go (2)
3-8: Good organization of imports for error handling.The added imports provide all the necessary dependencies for the improved error handling mechanism, including Temporal's error types and the custom error utilities.
15-32: Well-documented error handling function with clear purpose.Thank you for adding the requested documentation. The
handleWorkflowErrorfunction effectively centralizes Temporal workflow error handling by properly identifying validation errors and wrapping them with the appropriate error type.The use of
errorsutils.Cause(err)helps extract the root cause of the error, improving error clarity by removing unnecessary wrapping layers while preserving the essential context.#!/bin/bash # Check for usage of the function in the codebase echo "Looking for usage of handleWorkflowError:" rg -A 2 "handleWorkflowError" --type go echo -e "\nChecking for test coverage:" find . -name "*_test.go" | grep -i "engine/errors" || echo "No dedicated test file found"🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 17-27: internal/connectors/engine/errors.go#L17-L27
Added lines #L17 - L27 were not covered by tests
[warning] 31-31: internal/connectors/engine/errors.go#L31
Added line #L31 was not covered by testsinternal/connectors/plugins/public/currencycloud/client/auth.go (2)
10-11: Good addition of the errorsutils importThe addition of the errorsutils package aligns with the PR objective of improving error handling for connectors.
40-44:❓ Verification inconclusive
Improved error handling structure
The switch from using a simple
fmt.Errorftoerrorsutils.NewWrappedErrorenhances error handling by preserving the original error context while providing a clearer error message. This aligns well with the PR objective of improving connector error messaging.Note that the error message now only includes the
errRes.Error()content, which should provide more focused information about the failure.Since these lines aren't covered by tests according to codecov, consider ensuring this error handling works as expected:
🏁 Script executed:
#!/bin/bash # Check if there are any tests covering the authenticate method rg -A 2 -B 2 "authenticate.*Test" --type goLength of output: 45
Action Required: Verify Test Coverage for Authenticate Error Handling
The updated error handling using
errorsutils.NewWrappedErroreffectively preserves the original error context while simplifying the error message. However, our search for tests covering theauthenticatemethod returned no matches. Please verify manually whether there are any tests (or plans to add them) for this workflow. If tests are indeed missing, consider adding them to ensure that this error handling works as intended.
- File:
internal/connectors/plugins/public/currencycloud/client/auth.go- Lines: 40-44
- Diff Context:
return errorsutils.NewWrappedError( fmt.Errorf("failed to get authenticate: %v", errRes.Error()), err, )🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 40-43: internal/connectors/plugins/public/currencycloud/client/auth.go#L40-L43
Added lines #L40 - L43 were not covered by testsinternal/connectors/plugins/public/modulr/client/error.go (3)
4-4: Good addition of the errors importThe import is necessary for the improved error handling implementation.
8-9: Improved error type structureChanging from a single error to an array of errors provides more flexibility for handling multiple error conditions from the Modulr API.
19-25: Enhanced error handling for multiple errorsThe method signature change from a pointer to a single error to a slice of errors is a good improvement. The added check for an empty error slice is especially important to avoid potential panic situations.
This change aligns with the PR objective of improving error messaging for connectors by making the error handling more robust.
internal/connectors/plugins/public/currencycloud/client/beneficiaries.go (2)
10-10: Good addition of the errorsutils importThe addition of the errorsutils package aligns with the PR objective of improving error handling for connectors.
56-60:❓ Verification inconclusive
Improved error handling structure
Similar to the auth.go file, the switch from using a simple
fmt.Errorftoerrorsutils.NewWrappedErrorenhances error handling by preserving the original error context while providing a clearer error message. This is consistent with the PR objective of improving connector error messaging.Note that the error message now only includes the
errRes.Error()content, which should provide more focused information about the failure.Since these lines aren't covered by tests according to codecov, consider ensuring this error handling works as expected:
🏁 Script executed:
#!/bin/bash # Check if there are any tests covering the GetBeneficiaries method rg -A 2 -B 2 "GetBeneficiaries.*Test" --type goLength of output: 49
Manual Verification Required: Ensure Test Coverage for GetBeneficiaries
The updated error handling in
internal/connectors/plugins/public/currencycloud/client/beneficiaries.go(lines 56–60) now useserrorsutils.NewWrappedErrorto wrap the original error, which aligns well with our goal of preserving error context and providing clearer messages.• However, a search for tests covering the
GetBeneficiariesmethod returned no matches.
• Please manually verify that when the error path is triggered, the new error handling behaves as expected.
• If feasible, consider adding dedicated tests for this functionality to secure coverage and future stability.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 56-59: internal/connectors/plugins/public/currencycloud/client/beneficiaries.go#L56-L59
Added lines #L56 - L59 were not covered by testsinternal/connectors/plugins/public/adyen/client/client.go (3)
15-15: Good addition of the errorsutils importThe addition of the errorsutils package aligns with the PR objective of improving error handling for connectors.
75-75: Improved error wrapping for rate limiting errorsGreat refactoring to use
errorsutils.NewWrappedErrorinstead offmt.Errorffor HTTP 429 (Too Many Requests) errors. This preserves the original error context while still indicating the rate limiting issue.This change aligns with the PR objective of enhancing error handling during connector operations.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 75-75: internal/connectors/plugins/public/adyen/client/client.go#L75
Added line #L75 was not covered by tests
79-79: Improved error wrapping for client errorsGreat refactoring to use
errorsutils.NewWrappedErrorinstead offmt.Errorffor HTTP client errors (400-499). This preserves the original error context while still indicating the client error nature.This approach provides better structured error information that can be more effectively handled by the caller.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 79-79: internal/connectors/plugins/public/adyen/client/client.go#L79
Added line #L79 was not covered by testsinternal/connectors/plugins/public/currencycloud/client/contacts.go (3)
12-12: Good import addition for consistent error handlingAdding the errorsutils package aligns with the PR's objective of improving connector error handling throughout the codebase.
50-54:❓ Verification inconclusive
Better error classification for invalid requests
Wrapping the error with
models.ErrInvalidRequestproperly classifies this as a client-side validation error rather than a server error, which will result in returning a BadRequest (400) response instead of an Internal Server Error (500). This directly addresses the PR objective of "sending a BadRequest response instead of an internal server error when a request fails due to user input."
🏁 Script executed:
#!/bin/bash # Check test coverage for this error path # Look for tests that exercise this validation error scenario rg -A 5 -B 5 "no contact found for account" --type go --glob "*test.go"Length of output: 73
Action Required: Confirm Test Coverage for Error Classification Behavior
The updated error handling—with errors being wrapped with
models.ErrInvalidRequestto ensure a BadRequest (400) response—is implemented correctly. However, our initial search for test cases that validate this error path produced no results. Please manually verify that a test exists (or add one) to confirm that when no contact is found, the API returns a 400 response instead of a 500.
- Code Location:
internal/connectors/plugins/public/currencycloud/client/contacts.go(lines 50-54)- Recommendation: Ensure there is a test confirming this error scenario; if not, add one to cover the client-side validation error.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 50-53: internal/connectors/plugins/public/currencycloud/client/contacts.go#L50-L53
Added lines #L50 - L53 were not covered by tests
43-47:❓ Verification inconclusive
Improved error handling with structured wrapper
This change enhances error handling by using
NewWrappedErrorto wrap the original error with a more descriptive message that includes the specific error response from CurrencyCloud. This aligns with the PR objective to improve connector error messaging.
🏁 Script executed:
#!/bin/bash # Check test coverage for this error path # Look for tests that exercise this error scenario rg -A 5 -B 5 "failed to get contacts" --type go --glob "*test.go"Length of output: 67
Enhanced Error Handling with Structured Wrapper – Verify Test Coverage
The update in
internal/connectors/plugins/public/currencycloud/client/contacts.go(lines 43–47) clearly improves error clarity by wrapping the original error with a detailed message viaNewWrappedError. However, our initial search for tests that exercise the “failed to get contacts” error path did not yield any results. Please verify manually or consider adding targeted tests to ensure this error handling behavior is properly exercised and maintained.
- File:
internal/connectors/plugins/public/currencycloud/client/contacts.go(lines 43–47)- Improved error wrapping with
NewWrappedErroris correctly implemented.- Action Required: Manually verify or add tests to cover this error scenario to avoid potential regressions.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 43-46: internal/connectors/plugins/public/currencycloud/client/contacts.go#L43-L46
Added lines #L43 - L46 were not covered by testsinternal/connectors/plugins/public/mangopay/client/payout.go (3)
11-11: Updated import path for error utilitiesThe import path has been updated to use an internal version of the error utilities, ensuring consistency across the codebase.
65-72: Enhanced error handling with detailed API error informationThe changes enhance error handling by:
- Adding
errResto capture detailed error information from Mangopay API- Using
NewWrappedErrorto wrap the original error with a more descriptive message that includes API-specific error detailsThis aligns with the PR objective to provide better error messages for connectors, helping with debugging and troubleshooting.
89-93: Improved error context with status code informationThis change enhances error handling by including the HTTP status code in the error message, providing valuable context for debugging API connection issues. The structured wrapping with
NewWrappedErrorensures the original error is preserved in the error chain.internal/connectors/plugins/public/moneycorp/client/transactions.go (3)
14-14: Good import addition for consistent error handlingAdding the errorsutils package aligns with the PR's objective of improving connector error handling throughout the codebase.
103-107: Improved error handling with structured wrapperThis change enhances error handling by using
NewWrappedErrorto wrap the original error with a more descriptive message that includes the specific error response from Moneycorp. This aligns with the PR objective to improve connector error messaging.
100-100:✅ Verification successful
Verify the error type change from singular to plural
The error variable type has changed from
moneycorpErrortomoneycorpErrors, suggesting a different error structure is expected from the API response.
🏁 Script executed:
#!/bin/bash # Check the definition of both error types to understand the difference # Find the definition of moneycorpError echo "Looking for moneycorpError definition:" rg -A 10 "type moneycorpError" --type go # Find the definition of moneycorpErrors echo "Looking for moneycorpErrors definition:" rg -A 10 "type moneycorpErrors" --type goLength of output: 2783
Confirmed error type change:
The change frommoneycorpErrortomoneycorpErrorsis intentional and correct. The newmoneycorpErrorstype is defined as a struct that aggregates multiplemoneycorpErrorinstances (via itsErrorsslice) and provides anError()method to return a formatted error. This aligns with the expected response structure from the API. No further modifications are needed.internal/connectors/plugins/public/atlar/bank_account_creation.go (3)
8-8: Good import addition for consistent error handlingAdding the errorsutils package aligns with the PR's objective of improving connector error handling throughout the codebase.
14-18: Improved validation error classificationThese changes correctly wrap validation errors with
models.ErrInvalidRequest, ensuring they're properly classified as client-side validation errors rather than server errors. This will result in returning a BadRequest (400) response instead of an Internal Server Error (500), directly addressing the PR objective.Also applies to: 22-26, 29-33
50-55: Action Required: Add Unit Tests for the Empty API Response ScenarioThe change correctly wraps the error message ("PostV1CounterParties: unexpected empty response") with
models.ErrFailedAccountCreation, clearly indicating the failure context. However, our static analysis confirms that this error path still lacks associated test coverage. The search only revealed the error log in the implementation, but no tests simulate an empty API response.
- Next Steps: Please add a unit test that simulates an empty API response to validate that the error wrapping is triggered as expected.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 50-54: internal/connectors/plugins/public/atlar/bank_account_creation.go#L50-L54
Added lines #L50 - L54 were not covered by testsinternal/connectors/plugins/public/bankingcircle/transfers.go (9)
11-11: Good addition of error utilities import.This import enables the new structured error handling approach throughout the file.
16-19: Improved error handling structure.The change from
fmt.Errorftoerrorsutils.NewWrappedErrorprovides a more structured way to handle errors. This approach makes it clearer that the error is related to an invalid request, which aligns with the PR objective of sendingBadRequestresponses instead of internal server errors for user input issues.
23-26: Consistent error handling pattern applied.Good job maintaining consistency with the error handling approach across similar validation cases.
56-59: Consistent error structure for source account retrieval errors.The error now clearly indicates the failure is related to retrieving the source account while maintaining the appropriate error type.
62-65: Structured error for missing source account identifiers.Good error message that clearly explains what's missing from the request.
71-74: Consistent error structure for destination account errors.Maintains the same structured approach for destination account retrieval errors.
77-80: Structured error for missing destination account identifiers.This completes the consistent error handling approach across all validation checks in the method.
47-50:❓ Verification inconclusive
Structured error handling for amount conversion failures.
This error pattern now consistently provides both the specific error context and appropriate error type.
According to the static analysis results, this error path isn't covered by tests:
🏁 Script executed:
#!/bin/bash # Look for tests that might exercise this error case grep -r "GetStringAmountFromBigIntWithPrecision" --include="*_test.go" .Length of output: 326
Subject: Verify Error Path Coverage for Amount Conversion in BankingCircle Transfers
The error wrapping in
transfers.go(lines 47-50) is implemented correctly and consistently provides context withmodels.ErrInvalidRequest. However, static analysis shows that this specific error branch isn’t directly exercised by tests in the BankingCircle context. Although the underlying functionGetStringAmountFromBigIntWithPrecisionis tested ininternal/connectors/plugins/currency/amount_test.go, please add or update tests—ideally within the BankingCircle module—to explicitly trigger and verify the error path intransfers.go.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 47-50: internal/connectors/plugins/public/bankingcircle/transfers.go#L47-L50
Added lines #L47 - L50 were not covered by tests
39-42:❓ Verification inconclusive
Enhanced error context for currency and precision errors.
This structured approach provides better context about the specific issue while maintaining the error type information.
The static analysis indicates this error path isn't covered by tests. Consider adding test cases that trigger this specific error condition:
🏁 Script executed:
#!/bin/bash # Find existing tests for this file and check coverage fd --type f "bankingcircle_test.go" --exec grep -l "TestValidateTransferRequest\|TestCreateTransfer" {} \;Length of output: 109
Manual Verification Needed for Test Coverage on Enhanced Error Path
The updated error handling in
internal/connectors/plugins/public/bankingcircle/transfers.go(lines 39-42) indeed improves context by wrapping the original error with more detailed information. However, the script used to search for tests (bankingcircle_test.golooking forTestValidateTransferRequestorTestCreateTransfer) did not return any results. This raises a concern that this specific error path might not be covered by automated tests.Please manually verify whether there are tests that trigger this condition. If none exist, consider adding test cases to ensure that scenarios where currency and precision retrieval fails are properly validated.
#!/bin/bash # Manual verification for test coverage in bankingcircle_test.go fd --type f "bankingcircle_test.go" --exec grep -l "TestValidateTransferRequest\|TestCreateTransfer" {} \;internal/connectors/plugins/public/bankingcircle/client/payments.go (4)
11-11: Good addition of error utilities import.This import enables the new structured error handling approach throughout the file.
123-126: Improved error wrapping for payment listing.Changed from using simple
fmt.Errorfto a more structured approach witherrorsutils.NewWrappedError. This provides better error context by separating the operation error message from the underlying error.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 123-126: internal/connectors/plugins/public/bankingcircle/client/payments.go#L123-L126
Added lines #L123 - L126 were not covered by tests
147-150:❓ Verification inconclusive
Consistent error handling for payment retrieval.
The structured error approach is consistently applied to the
GetPaymentmethod, improving error clarity.The static analysis indicates this error path isn't covered by tests. Please verify test coverage:
🏁 Script executed:
#!/bin/bash # Check if there are tests for GetPayment method grep -r "GetPayment" --include="*_test.go" . | grep -v "GetPayments"Length of output: 1385
Review Update: Verify Explicit Error-Path Test Coverage for GetPayment
The implementation for structured error handling in the
GetPaymentmethod is consistent and clear. However, while several tests invokeGetPayment(as seen in the outputs frompayouts_test.go,transfers_test.go, and the e2e tests), these tests focus on the successful path. There isn’t clear evidence that the error branch—where the method returns a wrapped error—is explicitly exercised.
- Actionable Item: Please verify (or add) tests specifically triggering and asserting the error path in
GetPayment(e.g., by simulating a failure scenario that produces a non-200 status code and checking for the correct wrapped error).🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 147-150: internal/connectors/plugins/public/bankingcircle/client/payments.go#L147-L150
Added lines #L147 - L150 were not covered by tests
174-177:❓ Verification inconclusive
Standardized error handling for payment status retrieval.
The error handling approach is consistently applied across all methods in this file.
The static analysis indicates this error path isn't covered by tests. Consider adding tests:
🏁 Script executed:
#!/bin/bash # Check if there are tests for GetPaymentStatus method grep -r "GetPaymentStatus" --include="*_test.go" .Length of output: 50
Action Required: Verify test coverage for GetPaymentStatus error handling
The error handling logic at lines 174–177 in
internal/connectors/plugins/public/bankingcircle/client/payments.go
adheres to our standardized approach across methods. However, our initial grep search for references (using the pattern "GetPaymentStatus" in *_test.go files) produced no output. This makes it unclear whether the error path for this method has been tested.
- Location: internal/connectors/plugins/public/bankingcircle/client/payments.go (lines 174–177)
- Recommendation: Please verify manually whether tests exist for the GetPaymentStatus error scenario. If they don’t, consider adding tests to ensure that error cases (for example, when a non-success status code is returned) are properly covered.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 174-177: internal/connectors/plugins/public/bankingcircle/client/payments.go#L174-L177
Added lines #L174 - L177 were not covered by testsinternal/connectors/engine/activities/errors.go (2)
11-11: Good addition of error utilities import.This import enables the new error cause extraction approach throughout the file.
33-36: Well-documented error handling improvement.The comment clearly explains why we're extracting only the cause of the error - to prevent sending the entire stack trace to Temporal when returning from an activity.
internal/connectors/engine/engine.go (5)
18-18: Good addition of error utilities import.This import enables the new structured error handling approach throughout the file.
120-120: Improved error structure for validation failures.Changed from
errors.Wraptoerrorsutils.NewWrappedErrorfor validation errors, providing a clearer error structure.
138-138: Consistent error handling for plugin registration validation errors.Good job applying the structured error handling approach consistently throughout the method.
685-685:❓ Verification inconclusive
Consistent workflow error handling in CreatePayout.
The same improved error handling approach has been correctly applied to the CreatePayout method.
This error path also isn't covered by tests according to static analysis:
🏁 Script executed:
#!/bin/bash # Check for existing tests for CreatePayout that would reach this error path grep -r "TestCreatePayout" --include="*_test.go" .Length of output: 50
Please verify that the error path in CreatePayout is exercised by tests or through integration.
- The improved error handling—returning
models.Task{}, handleWorkflowError(err)—has been applied to the CreatePayout method consistently.- However, our static analysis search for tests referencing “TestCreatePayout” returned no results. This indicates that the error path might not be covered by the current test suite.
- I recommend manually verifying whether this scenario is actually exercised (or adding a dedicated test if it’s a concern).
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 685-685: internal/connectors/engine/engine.go#L685
Added line #L685 was not covered by tests
567-567:❓ Verification inconclusive
Updated error handling for workflow execution.
The change from directly returning the error to using
handleWorkflowErrorimproves error handling consistency for workflow-related errors.This error path isn't covered by tests according to static analysis:
🏁 Script executed:
#!/bin/bash # Check for existing tests for CreateTransfer that would reach this error path grep -r "TestCreateTransfer" --include="*_test.go" .Length of output: 52
Action: Verify test coverage for the new error handling path
The update to use
handleWorkflowErrordoes streamline our error reporting for workflow execution errors. However, our static check (via thegrep -r "TestCreateTransfer" --include="*_test.go" .command) didn’t locate any tests covering this error path. Please manually verify whether this error scenario is exercised during testing or consider adding a dedicated test case to ensure this behavior is properly validated.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 567-567: internal/connectors/engine/engine.go#L567
Added line #L567 was not covered by tests
Description
What this PR do
Before
FCTL
ADJUSTMENTS
V3 TASKS
Same as adjustments above
After
FCTL
ADJUSTMENTS
V3 TASKS
Fixes PMNT-76
Fixes PMNT-77