Right now the 7.x-3.x version of the Recurly.js module doesn't display any useful information when a credit card is declined. On the subscribe form you'll get a PHP error in most cases, and at best you'll get the message "Unable to create subscription." with no additional details. On the billing info update form you'll likely trigger an exception.

We should add better exception handling for Recurly_ValidationError exceptions, and display the message we get from the gateway to the user. These errors are generally user correctable things like invalid address, wrong CC number, etc. By not showing them to the user we're likely going to end up driving people away who don't know why their transaction is declined and are un-willing to contact you to ask for more information.

CommentFileSizeAuthor
#13 Screen Shot 2017-04-10 at 2.17.16 PM.png17.95 KBadamzimmermann
#13 display_error_message-2854667-13.patch3.33 KBadamzimmermann
#10 interdiff-7-10.txt930 bytesmarkdorison
#10 display_error_message-2854667-10.patch2.98 KBmarkdorison
#9 interdiff-7-9.txt924 bytesmarkdorison
#9 display_error_message-2854667-9.patch2.98 KBmarkdorison
#7 display_error_message-2854667-7.patch2.98 KBmarkdorison
#2 2854667-2-cc_validation.patch3.04 KBeojthebrave
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

eojthebrave created an issue. See original summary.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
CreditAttribution: eojthebrave at Drupalize.Me commented
Status: Active » Needs review
FileSize
2854667-2-cc_validation.patch3.04 KB

This patch adds exception handling for credit card form validation errors. Generally speaking this is handling errors related to the issuing bank or gateway declining the card or a transaction.

You can test the various error codes using the test CC numbers with the Recurly test gateway - https://docs.recurly.com/docs/test

In recurlyjs_subscribe_form_submit() I removed the line $recurly_account->create(); that fell outside the try/catch block. This was causing a Recurly_ValidationError exception to be thrown if a card is declined, and the request to be aborted because we're not handling the exception. The call is also not necessary since just below we call $subscription->create() which will also save the account info.

It also adds exception handling for Recurly_ValidationError to both the subscription form and billing info update form. I chose to not log these errors using watchdog after displaying them because the declined transaction, and all it's details, are already logged in Recurly. Though, we could add it to watchdog as well if that's a good idea?

adamzimmermann’s picture

adamzimmermann CreditAttribution: adamzimmermann at Chromatic commented
Status: Needs review » Reviewed & tested by the community

I installed this patch and tried several invalid card numbers, commented out some token code, and all the errors were handled. I can also confirm that accounts are still created, even with $recurly_account->create(); removed.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Reviewed & tested by the community » Fixed
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Version: 7.x-3.x-dev » 8.x-1.x-dev
Status: Fixed » Patch (to be ported)
Issue tags: +needs port to 8.x-1.x
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Patch (to be ported) » Needs review
FileSize
display_error_message-2854667-7.patch2.98 KB
adamzimmermann’s picture

adamzimmermann CreditAttribution: adamzimmermann at Chromatic commented
Status: Needs review » Needs work 
+++ b/modules/recurlyjs/src/Form/RecurlyJsUpdateBillingForm.php
@@ -118,8 +118,22 @@ class RecurlyJsUpdateBillingForm extends RecurlyJsFormBase {
+        \Drupal::logger('recurlyjs')->error('Billing information update error: @error', ['@error' => $e->getMessage()]);

I didn't test this yet, but I did notice one thing. \Drupal::logger('recurlyjs') should be called by injecting this service, as opposed to using the \Drupal class.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs work » Needs review
FileSize
display_error_message-2854667-9.patch2.98 KB
interdiff-7-9.txt924 bytes

@adamzimmermann Good catch; I cleaned up the instance present in this diff. We should create an independent ticket to resolve this elsewhere throughout the module.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
FileSize
display_error_message-2854667-10.patch2.98 KB
interdiff-7-10.txt930 bytes

Bad patch; correction attached.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
adamzimmermann’s picture

adamzimmermann CreditAttribution: adamzimmermann at Chromatic commented
Assigned: Unassigned » adamzimmermann
adamzimmermann’s picture

adamzimmermann CreditAttribution: adamzimmermann at Chromatic commented
Issue summary: View changes
FileSize
display_error_message-2854667-13.patch3.33 KB
Screen Shot 2017-04-10 at 2.17.16 PM.png17.95 KB

I re-rolled this patch to fix a conflict, and also removed $recurly_account->create(); to match the D7 changes. Without this removed, the form still throws errors, and removing it seems to be safe based upon the discussion above. I can confirm that I see an error when I use 4000-0000-0000-0002 as a card number.

So this is tested, but I'll let someone else mark it RTBC, as I don't like to mark my own patches as reviewed without someone else confirming they work as intended.

adamzimmermann’s picture

adamzimmermann CreditAttribution: adamzimmermann at Chromatic commented
Assigned: adamzimmermann » Unassigned

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.