-
Notifications
You must be signed in to change notification settings - Fork 24
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
[SDP-1397] Circle Payouts pt2: error handling & retries #491
Conversation
stellar-disbursement-platform-backend-preview is available here: |
stellar-disbursement-platform-backend-preview is available here: |
stellar-disbursement-platform-backend-preview is available here: |
a1d17ec
to
e43fafa
Compare
stellar-disbursement-platform-backend-preview is available here: |
e43fafa
to
f15f82b
Compare
stellar-disbursement-platform-backend-preview is available here: |
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.
LGTM! This is great!
_, err = p.Models.CircleRecipient.ResetRecipientsForRetryIfNeeded(ctx, dbTx, reqBody.PaymentIDs...) | ||
if err != nil { | ||
return nil, fmt.Errorf("resetting circle recipients for retry if needed: %w", err) | ||
} |
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.
nit: Totally optional as it's not an expensive operation. But you can also add a check to only do this operation for Circle distribution accounts.
distAccount, err := p.DistributionAccountResolver.DistributionAccountFromContext(ctx)
if err != nil {
return nil, fmt.Errorf("resolving distribution account: %w", err)
}
if distAccount.IsCircle() {
_, err = p.Models.CircleRecipient.ResetRecipientsForRetryIfNeeded(ctx, dbTx, reqBody.PaymentIDs...)
if err != nil {
return nil, fmt.Errorf("resetting circle recipients for retry if needed: %w", err)
}
}
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.
Updated in 83469c5.
internal/services/paymentdispatchers/circle_payment_dispatcher.go
Outdated
Show resolved
Hide resolved
internal/services/paymentdispatchers/circle_payment_dispatcher.go
Outdated
Show resolved
Hide resolved
…tore the error in the response_body column.
stellar-disbursement-platform-backend-preview is available here: |
stellar-disbursement-platform-backend-preview is available here: |
Something went wrong with PR preview build please check |
stellar-disbursement-platform-backend-preview is available here: |
What
Errors in the POST /recipient endpoint are not being properly handled. What;s missing?
ErrCircleRecipientCreationFailedTooManyTimes
, we update the payments table with the failed status explaining the reason.status=NULL
andsync_attempts=0
.ensureRecipientIsReady
was split into different sub-methods for better modularity and clarity, as suggested in [SDP-1397] Circle payouts pt1 #486 (comment).Why
To wrap up https://stellarorg.atlassian.net/browse/SDP-1397
Checklist
PR Structure
Thoroughness
Configs and Secrets
values.yaml
file.pr-preview
,dev
,demo
,prd
).values.yaml
file.pr-preview secrets
,dev secrets
,demo secrets
,prd secrets
).Release
develop
ormain
after it's ready for production!Deployment