Failed invoice payments are not retried

[Ying Que]

Hi There,

I have built a custom made java payment plugin based on Adyen plugin but this is for the context of bank to bank transfer - direct debit

and it takes several days to know if succeed or fail.

So my code sets the payments and transactions to PENDING first

but also, I notice the payment_attempts is marked as SUCCESS 

This is different to credit cards done by Stripe or Adyen where you know the results straight away 

in fact when my plugin fails in the first step can not get into pending the retries do kick in as configured 

Then when I poll the external paymnent gateway and obtain

1. (from pending) to success, no issues, I use transitionPendingTransaction that is simple

2. (from pending) to failure, I was using fixPaymentTransactionState because it allows me to copy error code and error message into the payment_transactions table but it is completing the payment_attempt and marking it as aborted

I then switched to use transitionPendingTransaction it was still marking the payment_attempt as aborted (not allowing copy of error_code and error_message) 

How should the payment_attempts work in such scenario

I have spend several weeks on and off looking around the killbill 0.20.0 code base 

and I can find retrystates and paymentstates xml and other code 

seems to me an OperationResult == EXCEPTION would lead to ABORTED

but it should be FAILURE instead

I also looked into the GitHub project of paymentControlPlugin but that seems to register retry counts based on error code

Any suggestions to me ?

1. Do I need to write my own PaymentControlPlugin ?

2. Or I need to "register" my plugin for retry I am not a ruby developer so I am not sure how Stripe plugin works but as I said above I do not think that is the issue

I tend to think my peculiar problem is I need to send a request to my bank to bank Direct Debit then poll for a result (they have no webhook or other callbacks)

3. is The lack of callBack an issue here do you think? I need to replace Polling with a callback ?

Thanks

Ying

[stephane brossier]

There is a lot of information and not always the clarity needed to really follow your steps -- if you want to go in this level of details, please make reference to exact api you are using -- e.g not sure what transitionPendingTransaction is?

Questions:

• Are you making payments in the context of subscription billing or using our raw payment apis (no subscription/invoice) ?
• Which version are you using -- are you using 0.20.0 , why not 0.20.10 ? Note that we typically provide mailing list support for latest stable version, otherwise this gets too mind bending for us.

Some info that might help: When you use payment attempts, the state machine defines only 3 states:

• RETRIED: A plugin decided to retry a failed operation
• ABORTED: A plugin decided to abort the call prior it was made
• SUCCESS: Anything else. Your low level transactional PENDING state will therefore mark the attempt as SUCCESS

Stéphane

--
You received this message because you are subscribed to the Google Groups "Kill Bill users mailing-list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to killbilling-us...@googlegroups.com.
To post to this group, send email to killbill...@googlegroups.com.
Visit this group at https://groups.google.com/group/killbilling-users.
To view this discussion on the web visit https://groups.google.com/d/msgid/killbilling-users/28b2abd0-5b61-404f-82f6-2fa0be80a8e0%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Ying Que]

Hi Stephane,

After looking deeper and trying this again and again 

I found the root cause to my issue of failed payments not retried and I am attaching your latest code fresh from GitHub 

At https://github.com/killbill/killbill/blob/master/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentAttemptTask.java 

final boolean isApiPayment = true; // unclear

Why is the above line hardcoding to true.

Later after a series of calls 

At 

https://github.com/killbill/killbill/blob/master/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentControlPluginApi.java

The below function is always exiting prematurely leading the OperationResult == EXCEPTION consequently the Control Plugin aborts the payment attempt 

private DateTime computeNextRetryDate(final String paymentExternalKey, final boolean isApiAPayment, final InternalCallContext internalContext) {
	// Don't retry call that come from API.
	if (isApiAPayment) {
	return null;
	}

Can you elaborate why this was done this way 

Because I am inclined to set isApiPayment to false and I shall check a bit more to see if that will have other adverse impact on the code base 

Simply put, if my payment is a credit card charge that gets an instant result fail or success from stripe , retries work 

If my payment is a inter-bank EFT (Direct Debit) then the retries are aborted when I my payment plugin call 

paymentApiWrapper.transitionPendingTransaction(***);

Given the 0.20.10 code in these places are unchanged from 0.20.0 I am confident this issue exists in your latest version but not many countries do these inter bank EFT in my country it’s still prevalent though we are moving to an nation wide instant bank transfer platform to rival credit card but till that s done I need to solve this issue. 

On Thu, Jul 4, 2019 at 3:54 AM stephane brossier <step...@kill-bill.org> wrote:

There is a lot of information and not always the clarity needed to really follow your steps -- if you want to go in this level of details, please make reference to exact api you are using -- e.g not sure what transitionPendingTransaction is?

I was calling paymentApiWrapper.transitionPendingTransaction(***);

Questions:

• Are you making payments in the context of subscription billing or using our raw payment apis (no subscription/invoice) ? - recurring payments with invoices and subscription 

To view this discussion on the web visit https://groups.google.com/d/msgid/killbilling-users/CAKuqY30Bi51QG0NwYuOwP0xDOT5a2onFP194J-WdXZRVGY4TLg%40mail.gmail.com.

[stephane brossier]

The short story is the following:

• If a payment is made in the context of a bus event (e.g typical use case of recurring payment), we allow for retry
• If a payment is made in the context of any api call, we don't allow for retry because the caller will be notified of the error and can decide whether or not this is appropriate to retry

[Ying Que]

Hi Stephane,

Thank you for your straight to the point explanations which make sense.

However, as I tried to elaborate prior:

My failed payments are made in the context of a bus event as recurring payment triggered from a subscription.

I can only surmise that:

1. Subscription recurring payment is due and my java payment plugin is invoked to directly debit a bank account via an external payment provider.
2. The initial payment/payment transactions are marked as PENDING and payment attempt is marked as SUCCESS (from transaction being in PENDING)
3. In my case, most of inter-bank transfers take 2-3 business days to complete so we either poll or subscribe to a webhook from the external payment provider.
4. When a payment of such kind fails through polling or webhook notifications, I call paymentApiWrapper.transitionPendingTransaction (now is this perceived by your code base as an apiPayment call ?)
5. Rest is as you know due to that isAPIPayment being set to true in the IncompletePaymentAttemptTask.java no retries are scheduled.
   

 Without digging further, and the above affecting my live system, I have overridden your isAPIPayment variable to effectively make your second scenario allowing retries

We delegate the if retry decision to the configuration per Tenant in Killbill, we use multiple tenants in Killbill in our design. 

Thank you again for your help I have understood deeper into the inner working of KB through this journey.

Ying

[stephane brossier]

[...]

1. Subscription recurring payment is due and my java payment plugin is invoked to directly debit a bank account via an external payment provider.
2. The initial payment/payment transactions are marked as PENDING and payment attempt is marked as SUCCESS (from transaction being in PENDING)
3. In my case, most of inter-bank transfers take 2-3 business days to complete so we either poll or subscribe to a webhook from the external payment provider.
4. When a payment of such kind fails through polling or webhook notifications, I call paymentApiWrapper.transitionPendingTransaction (now is this perceived by your code base as an apiPayment call ?)

Yes absolutely - as a matter of fact any interaction from your plugin to KB core will need to happen through api calls. Which means that if such operation were to fail, this would not be retried. 

1. Rest is as you know due to that isAPIPayment being set to true in the IncompletePaymentAttemptTask.java no retries are scheduled.
   

 Without digging further, and the above affecting my live system, I have overridden your isAPIPayment variable to effectively make your second scenario allowing retries

We delegate the if retry decision to the configuration per Tenant in Killbill, we use multiple tenants in Killbill in our design. 

So i assume you had to fork the code and recompile with this change? 

Also, i am confused why can't you embed the retry logic in your plugin instead of hacking the code?

Stéphane

[Ying Que]

Thanks Stephane again for your patience with me and please see my comment in line:

On Tue, Jul 9, 2019 at 12:56 PM stephane brossier <step...@kill-bill.org> wrote:

[...]

1. Subscription recurring payment is due and my java payment plugin is invoked to directly debit a bank account via an external payment provider.
2. The initial payment/payment transactions are marked as PENDING and payment attempt is marked as SUCCESS (from transaction being in PENDING)
3. In my case, most of inter-bank transfers take 2-3 business days to complete so we either poll or subscribe to a webhook from the external payment provider.
4. When a payment of such kind fails through polling or webhook notifications, I call paymentApiWrapper.transitionPendingTransaction (now is this perceived by your code base as an apiPayment call ?)

Yes absolutely - as a matter of fact any interaction from your plugin to KB core will need to happen through api calls. Which means that if such operation were to fail, this would not be retried. - I am confused here:

1. Out of Box, Stripe credit card payments or my bank debit plugin payment failures are retried by the default payment control plugin, PROVIDED the failure status is returned in the synchronous response to the purchase transaction, in other words, if then payment completed and failed without delays.
2. If a PEDNING (IncompletePaymentAttemptTask.java is involved) payment is notified of a failure sometime later, then no retry by your design. These are two distinct scenarios IMO, both of which involve a plugin interacting with KB Core. 

Could I then interpret your clarification above as "any incomplete payments in-flight if failed will NOT be retried as per design"?

1. Rest is as you know due to that isAPIPayment being set to true in the IncompletePaymentAttemptTask.java no retries are scheduled.
   

 Without digging further, and the above affecting my live system, I have overridden your isAPIPayment variable to effectively make your second scenario allowing retries

We delegate the if retry decision to the configuration per Tenant in Killbill, we use multiple tenants in Killbill in our design. 

So i assume you had to fork the code and recompile with this change? - Yes I had to do so since this behaviour is unchanged from 0.20.0 to 0.20.10. 

Also, i am confused why can't you embed the retry logic in your plugin instead of hacking the code? - I followed your document on overdue invoices and found org.killbill.payment.retry.days:3,3,3. This worked perfectly for any payment failures completed instantly without delays. I decided to fork so the same retries apply to payments waiting for a callback to know if failed or succeeded.

 

If I may suggest, this behaviour of "If a payment is made in the context of any api call, we don't allow for retry because the caller will be notified of the error and can decide whether or not this is appropriate to retry" could be made configurable e.g., org.killbill.payment.retry.isapipaymentpermitted = false (default to false). But other users can override it to true.

 

But I totally understand if you do not agree. 

[Shaun Forgie]

Ying,

I am trying to follow you but am getting lost in the details....so firstly I will recap what I think you are wanting to do and where the ambiguities are so that we can then try to work through how best to implement this feature in Kill Bill. OK with you?

Bank Direct Debit Payment Processing

Objective: You want to build a payment plugin the produces direct debit instructions that get sent to the bank automated clearing house for settlement. Given the bank ACH facility can take 1-2 business days to transfer and settle the payment amounts you are not able to update the status of the payment immediately. Thus payment instruction status confirmation is required such that reconciliation can occur and the appropriate account and invoice activities can be finalised.

Q1. Are these instructions produced and sent individually to the bank for each invoice. You are NOT wanting to batch process all Accounts/Subscriptions having the same BCD into a single file which is then sent to the bank at the end of the day for ACH settlement.

Q2. How do you interface with the bank ACH facility. Is there a HTTP Rest API, a SOAP interface or a more traditional file transfer mechanism involved?

Q3. Does the bank provide direct debit instruction identifiers. If not how is a unique DD instruction to be reconcilled with bank account transactions?

Q3. How is the payment status of the direct debit instruction to be ascertained from the bank.  Do they send it to you. Do you request it from them. Do you have to poll for it?

Commentary:

It appears that the delayed settlement nature of direct debit instructions has similiarities with certain credit card payment scenarios . For example a scenario like a hotel booking often results in two or more payment transactions related to a given invoice payment. In this case the initial Authorisation and then subsequent Capture events occur at different points in time. Each of these payment transactions may in turn have a number Payment Attempts because of problems with the Payment Service Provider due to system faults and / or network connectivity interruptions.

ᐧ

To view this discussion on the web visit https://groups.google.com/d/msgid/killbilling-users/CABUAYrux95YFtcy4OagA49jWkro2H%3DdyCek6jsdp4O06pjSONQ%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Shaun Forgie [Principal] - Method Maker Ltd
PO Box 59135, Manukau 2151, Auckland, New Zealand
Mobile +64 21 666 910

[stephane brossier]

[...]

 

Yes absolutely - as a matter of fact any interaction from your plugin to KB core will need to happen through api calls. Which means that if such operation were to fail, this would not be retried. - I am confused here:

1. Out of Box, Stripe credit card payments or my bank debit plugin payment failures are retried by the default payment control plugin, PROVIDED the failure status is returned in the synchronous response to the purchase transaction, in other words, if then payment completed and failed without delays.
2. If a PEDNING (IncompletePaymentAttemptTask.java is involved) payment is notified of a failure sometime later, then no retry by your design. These are two distinct scenarios IMO, both of which involve a plugin interacting with KB Core. 

Could I then interpret your clarification above as "any incomplete payments in-flight if failed will NOT be retried as per design"?

No i don't think so. Whether a payment transaction status is PENDING (your bank transfer use case) or SUCCESS is treated the same: In both cases, this is a success state and there is no retry. Then, for cases of failures (again, whether for your bank transfer use case, or any stripe credit card use case), the story is simple: If it is from an api call we don't do automatic retries and if not (e.g async bus event) we do have a config-based retry logic in place.

We could certainly make this behavior configurable -- as you suggested -- but currently this is not supported.

Different topic: I would highly suggest you update to 0.20.10 which has a lot of bug fixes --incl. the latest one Pierre fixed for the BCD event you reported.

[Ying Que]

Is there an official guide on upgrade of existing live KB system ?

I googled it just now and find a migration link but that is from non KB to KB, not relevant.

I would see two immediate concerns:

1. my data, which is stored in a MySQL docker as per 0.20.0 config
2. some plugins which I have found for instance a tax plugin I did not use your OSS release recommended one that needed maven change to work with 0.20.0, it worked with 0.18.* and the folks I chatted to who used that plugin said they (in Early 2019) still using 0.18.*.

My current "lazy" strategy is I have latest KB installed on my PC as a local dev environment for me to evaluate potential bug fixes.

And I manually merge what I need from your LATEST HEAD into my fork of 0.20.0. That is NOT ideal hence please point me to a URL or email thread where I can find the info , much appreciated.

Thanks Stephane, on this retry topic, I am highly satisfied that now I can see my status poller makes api calls to your core module to update the final transaction status when the bank transfer is settled as SUCCESS or FAILURE.

By you design that is not a bus event and Beatrix listener is NOT listening to it hence the config-based retry logic I have relied on will not be invoked, till I changed that behaviour.

Yes I would appreciate if you can make this behaviour configurable in 0.20.10 that will add one more motivation to update

[stephane brossier]

Please see our release notes here for each version -- you need to read each set from 0.20.x -> 0.20y