Skip to content
Snippets Groups Projects

Issue #3112812: Add logging for payment failures during checkout

Merged Dmytrii Kaiun requested to merge issue/commerce-3112812:3112812-add-logging-for into 8.x-2.x

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 19 * The event dispatcher.
    20 */
    21 protected EventDispatcherInterface $eventDispatcher;
    22
    23 /**
    24 * The order.
    25 */
    26 protected ?OrderInterface $order = NULL;
    27
    28 /**
    29 * {@inheritdoc}
    30 */
    31 public static function create(ContainerInterface $container) {
    32 $instance = parent::create($container);
    33 $instance->eventDispatcher = $container->get('event_dispatcher');
    34 $instance->order = $container->get('current_route_match')->getParameter('commerce_order');
  • 83 109 $payment_gateway_plugin->createPaymentMethod($payment_method, $values['payment_details']);
    84 110 }
    85 111 catch (DeclineException $e) {
    112 if (
    113 $this->order instanceof OrderInterface &&
    114 $payment_method->getPaymentGateway() instanceof PaymentGatewayInterface
    115 ) {
    116 $event = new FailedPaymentEvent($this->order, $payment_method->getPaymentGateway(), $e);
    117 $this->eventDispatcher->dispatch($event, PaymentEvents::PAYMENT_FAILURE);
  • 180 154 $this->checkoutFlow->redirectToStep($next_step_id);
    181 155 }
    182 156 catch (DeclineException $e) {
    157 $event = new FailedPaymentEvent($this->order, $payment_gateway, $e, $payment);
    158 $this->eventDispatcher->dispatch($event, PaymentEvents::PAYMENT_FAILURE);
  • Dmytrii Kaiun added 3 commits

    added 3 commits

    Compare with previous version

  • Dmytrii Kaiun added 1 commit

    added 1 commit

    Compare with previous version

  • 165 165 public function submitInlineForm(array &$inline_form, FormStateInterface $form_state) {
    166 166 parent::submitInlineForm($inline_form, $form_state);
    167 167
    168 $payment_gateway = $this->entity->getPaymentGateway();
    169 $order = $this->routeMatch->getParameter('commerce_order');
  • 232 212 $this->checkoutFlow->redirectToStep($next_step_id);
    233 213 }
    234 214 catch (PaymentGatewayException $e) {
    215 $event = new FailedPaymentEvent($this->order, $payment_gateway, $e, $payment);
  • 145 147 ])->save();
    146 148 }
    147 149
    150 /**
    151 * Creates a log when payment failed.
    152 *
    153 * @param \Drupal\commerce_payment\Event\FailedPaymentEvent $event
    154 * The failed payment event.
    155 */
    156 public function onPaymentFailure(FailedPaymentEvent $event): void {
    157 $this->logStorage->generate($event->getOrder(), 'payment_failed', [
    158 'error_message' => $event->getGatewayException()->getMessage(),
    159 'gateway' => $event->getPaymentGateway()->label(),
    160 'amount' => $event->getPayment()?->getBalance(),
    161 ])->save();
    • Comment on lines +157 to +161

      Can we also add the payment id to this information. This would allow reports to build a much detailed picture of what is going on here. Maybe we can look at \Drupal\commerce_log\EventSubscriber\PaymentEventSubscriber::onPaymentInsert() as inspiration... so something like:

      Suggested change
      162 $this->logStorage->generate($event->getOrder(), 'payment_failed', [
      163 'error_message' => $event->getGatewayException()->getMessage(),
      164 'gateway' => $event->getPaymentGateway()->label(),
      165 'amount' => $event->getPayment()?->getBalance(),
      166 ])->save();
      162 $this->logStorage->generate($event->getOrder(), 'payment_failed', [
      163 'id' => $payment->id(),
      164 'remote_id' => $payment->getRemoteId(),
      165 'method' => $payment->getPaymentMethod()?->label(),
      166 'error_message' => $event->getGatewayException()->getMessage(),
      167 'gateway' => $event->getPaymentGateway()->label(),
      168 'amount' => $event->getPayment()?->getBalance(),
      169 ])->save();
    • I guess the remote ID will probably not be there but no harm in including it. The reason I think we need more info here is that we'd like to be able to tell which cards are being declined.

      Pushed a fix in my latest commit.

    • Alex Pott changed this line in version 18 of the diff

      changed this line in version 18 of the diff

    • Please register or sign in to reply
  • 19 * Constructs a new FailedPaymentEvent.
    20 *
    21 * @param \Drupal\commerce_order\Entity\OrderInterface $order
    22 * The order entity.
    23 * @param \Drupal\commerce_payment\Entity\PaymentGatewayInterface $paymentGateway
    24 * The payment gateway.
    25 * @param \Drupal\commerce_payment\Exception\PaymentGatewayException $gatewayException
    26 * The payment gateway exception.
    27 * @param \Drupal\commerce_payment\Entity\PaymentInterface|null $payment
    28 * The payment.
    29 */
    30 public function __construct(
    31 protected OrderInterface $order,
    32 protected PaymentGatewayInterface $paymentGateway,
    33 protected PaymentGatewayException $gatewayException,
    34 protected ?PaymentInterface $payment = NULL
    • What is the situation where we don't have a payment object. It feels really odd to not have one - so if this happens we should document why.

    • Alex Pott changed this line in version 22 of the diff

      changed this line in version 22 of the diff

    • Please register or sign in to reply
  • 72 98 $payment_method->save();
    73 99 }
    74 100 catch (DeclineException $e) {
    101 if (
    102 $this->order instanceof OrderInterface &&
    103 $payment_method->getPaymentGateway() instanceof PaymentGatewayInterface
    104 ) {
    105 $event = new FailedPaymentEvent($this->order, $payment_method->getPaymentGateway(), $e);
    106 $this->eventDispatcher->dispatch($event, PaymentEvents::PAYMENT_FAILURE);
    • Comment on lines +105 to +106

      Ah - here is a place without the payment - maybe we should be providing the payment method in some way here. So we can trace what is responsible for the error.

    • Alex Pott changed this line in version 22 of the diff

      changed this line in version 22 of the diff

    • Please register or sign in to reply
  • Alex Pott added 6 commits

    added 6 commits

    • fceaef13...e7f1f3f3 - 2 commits from branch project:8.x-2.x
    • 250c1fdd - Merge branch '8.x-2.x' into 3112812-add-logging-for
    • 615fae0e - Move to catch as suggested by @jsacksick
    • 33843a9c - refactor to use a single try/catch to make consistent logic easier to follow
    • dd741768 - Add more info to payment_failed log

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading