202-ecommerce / stripe_official

After years of hard work with Stripe connector for PrestaShop, 202 ecommerce stop the development of Stripe module on January 9th 2023. Thanks for all contributors that help us!
20 stars 20 forks source link

Webhook "conflict" with another payment mode #78

Closed Mindfield-Studio closed 2 years ago

Mindfield-Studio commented 3 years ago

Hi,

This morning I had a strange behavior:

Does webhook check transaction ID or another related thing ?

Thank you !

EDIT: Prestashop 1.7.4 and Stripe Module 2.3.5

Mindfield-Studio commented 3 years ago

Thank you ;)

Mindfield-Studio commented 3 years ago

After looking in the code, it seems it does not really fix the issue in every cases (even if the test is usefull): I think the main issue is the HTTP 400 error response when Stripe send the payment error to the webhook. I guess that if you use another card (a valid one) after a payment error, you can have a "Payment Error" anyway (same module, different card).

Mindfield-Studio commented 3 years ago

After some investigations, I think I found the bug. In 2.3.5, each action on Stripe is done twice, in "sync" mode (without the webhook), then in "async" mode (with the webhook). As far, it looks good, but:

I will work today on a fix, and I will post it here :)

EDIT: It seems it will be a bit longer to fix this, we have to check and store transaction state to know if we need to update the order or not.

Mindfield-Studio commented 3 years ago

For some HTTP 400 errors, in "ValidateOrderActions::chargeWebhook" :

$id_order = Order::getOrderByCartId($id_cart);
if ($id_order == false) {
    if ($this->conveyor['event_json']->type == 'payment_intent.requires_action') { // 3DSecure errors
        ProcessLoggerHandler::logInfo(
            'Unknown order requires action to process',
            null,
            null,
            'webhook'
        );
        ProcessLoggerHandler::closeLogger();
        http_response_code(200);
        return true;
    } else if ($this->conveyor['event_json']->type == 'charge.refunded') { // Refunded (without order)
                ProcessLoggerHandler::logInfo(
                    'Unknown order has been refunded',
                    null,
                    null,
                    'webhook'
                );
                ProcessLoggerHandler::closeLogger();
                http_response_code(200);
                return true;
    }  else if ($this->conveyor['event_json']->type == 'charge.failed') { // Charge failed (without order)
        ProcessLoggerHandler::logInfo(
            'Unknown order charge failed',
            null,
            null,
            'webhook'
        );
        ProcessLoggerHandler::closeLogger();
        http_response_code(200);
        return true;
    } else {
        ProcessLoggerHandler::logError(
            '$id_order = false',
            null,
            null,
            'webhook'
        );
        ProcessLoggerHandler::closeLogger();
        http_response_code(400);
        return false;
    }
}
clotairer commented 2 years ago

We have not encountered this issue during our test of 2.4.0 release.

Could you please confirm all 400 error case are now manage properly ?

Do not hesitate to suggest a code modification by sending us a Pull Request.