I've a situation (4-5 out 400 orders) not completed and order status set to "pending". I've checked through watchdog log and can confirm that all these failed orders do not receive any IPN request from paypal compared to other successful order where I can trace IPN request in watchdog and also in apache log. Through apache log I can see that url "car/echeckout/submit" being called but that's all. Customers then being redirected to "car/checkout/complete". Fortunately there's no transaction happened at paypal end (no money being transferred from customers to us) but it still look bad to customers because it look to them the order is completed.

When we email paypal, this is the response we got:-

Upon doec, paypal returns the server a response with paymentstatus which is what should be captured instead of only depending on the callback. can you ask your developer to parse the response in addition to the callback method.

So I take a look at payment/uc_paypal/uc_paypal.module and can see the following code:-

841	
842	  $nvp_response = uc_paypal_api_request($nvp_request, variable_get('uc_paypal_wpp_server', 'https://api-3t.sandbox.paypal.com/nvp'));
843	
844	  unset($_SESSION['TOKEN'], $_SESSION['PAYERID']);
845	  $_SESSION['do_complete'] = TRUE;
846	
847	  $form_state['redirect'] = 'cart/checkout/complete';
848	}

So the paypal response is never checkout and customers always being redirected to 'cart/checkout/complete' regardless of paypal payment status. So far, this is the workaround the I did:-

if ($nvp_response['ACK'] !== 'Success') {
    drupal_set_message("There's an error with the payment processor and your order cannot be completed. You are advised to call CLIENT office immediately. Your order number is $order->order_id", 'error');
    $error = implode(':', $nvp_response);
    watchdog('uc_paypal', '!error', array('!error' => $error), WATCHDOG_ERROR);
    $message = array(
      'to' => 'me@client.site',
      'subject' => 'Paypal Error',
      'body' => $error ." order_id: ". $order->order_id,
      'headers' => array('From' => 'system@client.site'),
    );
    drupal_mail_send($message);
    $_SESSION['do_complete'] = FALSE;
    $form_state['redirect'] = 'client/cart/error/'. $order->order_id;
  }
  else {
    unset($_SESSION['TOKEN'], $_SESSION['PAYERID']);
    $_SESSION['do_complete'] = TRUE;
    $form_state['redirect'] = 'cart/checkout/complete';
  }

This hopefully would catch the error so we can analyze it further because so far we left with nothing to tell client.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TR’s picture

Version: 6.x-2.2 » 6.x-2.x-dev
dsobon’s picture

Patch included.

Adds the following:
* debug for all request / responses from paypal.
* add missing error handling.

theamoeba’s picture

Hi dsobon,

The patch had to failures in uc_paypal.module at line 834 and 940.

longwave’s picture

Status: Active » Needs work
longwave’s picture

Patch needs work, the blank // comment lines are not needed, and _print_r_to_string() is not necessary either - just use print_r($data, TRUE).

dsobon’s picture

Version: 6.x-2.x-dev » 6.x-2.6

-dev?

I've tested it against 2.4 and 2.6 - official releases - and it applies fine.

dsobon’s picture

updated patch to include recommended changes as per post #5

TR’s picture

Version: 6.x-2.6 » 6.x-2.x-dev

Hi dsobon,

Thanks for working on this. It's looking really good.

The testbot isn't working at the moment, but if it were it would tell you that the patch is in the wrong format for automated testing. Drupal patches should be -p1 format, rooted at the base ubercart directory. Specifically, your patch starts off like this:

--- uc_paypal/uc_paypal.module.orig	2011-07-30 08:00:20.000000000 +1000
+++ uc_paypal/uc_paypal.module	2011-10-08 16:31:47.000000000 +1100

when it should look like this:

--- a/payment/uc_paypal/uc_paypal.module.orig	2011-07-30 08:00:20.000000000 +1000
+++ b/payment/uc_paypal/uc_paypal.module	2011-10-08 16:31:47.000000000 +1100

Likewise for the section patching uc_paypal.pages.inc.

Once this change is made, it applies properly to 6.x-2.x-dev, which is where we add new features like this. (Fixed-point releases like 6.x-2.6 are immutable - they will never receive this fix.) I've changed the version on this issue to reflect that.

I manually ran the patch through the SimpleTest cases, and it passed, but that doesn't mean much because we don't have any test cases for PayPal responses. I'll leave it to others to actually verify the functionality in this patch, as I don't use PayPal EC.

dsobon’s picture

Updated patch as per post post #8

TR’s picture

Status: Needs work » Needs review
TR’s picture

Status: Needs review » Needs work
TR’s picture

Status: Needs work » Needs review

Attempt to restart the bot...

TR’s picture

Status: Needs review » Needs work

And again ...

TR’s picture

Status: Needs work » Needs review
TR’s picture

Status: Needs review » Needs work
TR’s picture

Status: Needs work » Needs review
TR’s picture

Status: Needs review » Needs work

As you can tell, I'm using this issue as a means to debug the testbot. The end goal is to get the patch in #9 tested so we can get this fix into Ubercart. This issue just happens to be the most current one with a seemingly good patch for 6.x-2.x, so it's the most convenient one to use for testing. Sorry for the distraction here - hopefully I can get this figured out and we can get back to the main issue.

TR’s picture

Status: Needs work » Needs review
TR’s picture

Status: Needs review » Needs work
TR’s picture

Status: Needs work » Needs review
longwave’s picture

Status: Needs review » Needs work
longwave’s picture

Status: Needs work » Needs review
TR’s picture

Now that the testbot is working properly and has passed the patch in #9, I'd like to see a review from some PayPal users to make sure this doesn't break anything. This is an improvement I'd really like to see added.

jfarhat’s picture

FileSize
2.08 KB

Indeed there is an issue with the PayPal module, specifically in the Express Checkout API.

Symptoms:
When an Express Checkout process completes all orders succeeded, failed, or otherwise have a 'pending' status. For succeeded transactions the order details page doesn't show the PayPal transaction ID, the balance remains as the full order amount.

Source of the problem:
Tracing the issue we were able to narrow it down to the function uc_paypal_ec_submit_form_submit. We discovered that the PayPal response string is not processed at all.

Solution:
We processed the incoming response and made the appropriate calls to register the payment if succeeded, log the appropriate order comments. We found out that it worked beautifully.

Attached is a patch to solve the issue. Please let us know if that works out for you.

Joseph,
Kirkham Systems Inc.

Status: Needs review » Needs work

The last submitted patch, uc_paypal.patch, failed testing.

jfarhat’s picture

FileSize
2.08 KB

Patch resubmit with UTF-8

TR’s picture

Status: Needs work » Needs review

You have to set the issue status back to "needs review" in order to get the testbot to review it.

Status: Needs review » Needs work

The last submitted patch, uc_paypal.patch, failed testing.

sugarbase’s picture

Like the OP I have started getting some random PayPal Express orders moving to the 'pending' status without payment being completed.
Again nothing in the logs reporting the IPN & nothing on PayPals IPN history of the order too.

Currently double checking all completed orders match PayPal's 'payment received notification' emails prior to fulfilment, but it's not ideal. Also it will trigger the invoice & the customer may be lead to believe that the order has been completed.

Will any of the patches sort this or do I need to revert to PayPal WPS?

Drupal 6.26
Ubercart 6.x-2.9

Thanks.

longwave’s picture

There is nothing you can revert, this has apparently never worked properly. You can help by testing out the patches in this issue and letting us know if they work for you.

TR’s picture

Version: 6.x-2.x-dev » 8.x-4.x-dev

This needs to be addressed in D8 first, then backported to D7. D6 is obsolete, we won't be fixing anything in D6 anymore.