When cancelling a transaction on Sagepay's side, or indeed failing a payment by entering duff information, the POST back to Drupal results in the following errors:

Notice: Undefined index: AddressResult in commerce_sagepay_form_transaction() (line 448 of /data/drupal7/sites/my_site/site/sites/all/modules/contrib/commerce_sagepay_form/commerce_sagepay_form.module).
Notice: Undefined index: PostCodeResult in commerce_sagepay_form_transaction() (line 449 of /data/drupal7/sites/my_site/site/sites/all/modules/contrib/commerce_sagepay_form/commerce_sagepay_form.module).
Notice: Undefined index: AVSCV2 in commerce_sagepay_form_transaction() (line 450 of /data/drupal7/sites/my_site/site/sites/all/modules/contrib/commerce_sagepay_form/commerce_sagepay_form.module).
Notice: Undefined index: 3DSecureStatus in commerce_sagepay_form_transaction() (line 451 of /data/drupal7/sites/my_site/site/sites/all/modules/contrib/commerce_sagepay_form/commerce_sagepay_form.module).

I'm not sure if the Sagepay API changed recently - i only started seeing this in the last couple of weeks - but it appears that certain values are not returned when a payment fails, so the $tokens array has data missing.

I figure this could be fixed by a simple check to see if the data exists, however there may be a more appropriate way of handling the missing data in this situation?

Thanks

CommentFileSizeAuthor
#1 failed_transaction_missing_data_1777866.patch935 bytesrich.3po

Comments

rich.3po’s picture

rich.3po commented
StatusFileSize
new failed_transaction_missing_data_1777866.patch935 bytes

The attached patch fixes the errors, however I've not had much to get my head round the module as a whole so it may not be the best approach

rich.3po’s picture

rich.3po commented
Status: Active » Needs review
kiwimind’s picture

kiwimind commented

I've just noticed the same error.

Have you decided if this is the right approach yet or shall I apply and test the patch?

Thanks.

doliveros’s picture

doliveros commented

Yeah, I'm having the same issue. I'll apply and test the patch.

doliveros’s picture

doliveros commented

It does remove the notice messages. I believe it's the right approach, as those tokens aren't supposed to be there if the transaction is aborted.

ikos’s picture

ikos commented

Hi

Thanks for the patch. You are correct in that these variables should not be there in this scenario. There was a recent update to the SagePay API which could be the reason for this coming up now.

I'll give this and final test before committing.

Richard

ikos’s picture

ikos commented
Status: Needs review » Fixed

Hi,

I have committed the change and added a little to the patch in the process to carry out the same checks on the other returning variables in case they are a problem in the future,

Thanks again!

kind regards

Richard

Status: Fixed » Closed (fixed)

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