-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/adyen error objects #1228
base: feature/SFI-959-reliability-tracking
Are you sure you want to change the base?
Feature/adyen error objects #1228
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but we would need to consider the points below also, to ensure consistency:
- None of adyenDeleteRecurringPayment and deletePayment has a try/catch logic in them, meaning that if an error is thrown during deleting a stored payment method, we will not get any insight. I think we should apply the logic there also.
- zeroAuthPayment is inside a try catch , however it’s a helper function being used by savePayment(). Should we move try/catch logic in savePayment() instead?
- Should we consider adding an AdyenError in updateSavedCards()?
- Should we consider adding try/catch to notify() and remove try/catch on helper functions related to that. Instead we can replace them with AdyenError?
- Same as the above applies to begin.js file, removing the try/catch from the helpers and adding it to the main function.
throw new Error('No Customer ID or RecurringDetailReference provided'); | ||
} | ||
if (!(customerID && recurringDetailReference)) { | ||
throw new Error('No Customer ID or RecurringDetailReference provided'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this one be also AdyenError
?
@@ -26,12 +26,14 @@ const AdyenHelper = require('*/cartridge/adyen/utils/adyenHelper'); | |||
const AdyenConfigs = require('*/cartridge/adyen/utils/adyenConfigs'); | |||
const constants = require('*/cartridge/adyen/config/constants'); | |||
const AdyenLogs = require('*/cartridge/adyen/logs/adyenCustomLogs'); | |||
const setErrorType = require('*/cartridge/adyen/logs/setErrorType'); | |||
const { AdyenError } = require('*/cartridge/adyen/logs/adyenError'); | |||
|
|||
// eslint-disable-next-line complexity | |||
function donate(donationReference, donationAmount, orderToken) { | |||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove try/catch here, considering we are adding it at main function donation()
?
Quality Gate failedFailed conditions |
Summary
Describe the changes proposed in this pull request:
Tested scenarios
Description of tested scenarios:
Fixed issue: #SFI-1007 #SFI-1025