PRE-3472: Fix error 500 when user proceed to return_cancel action on scalapay checkout by jhoaraupp · Pull Request #304 · payplug/SyliusPayPlugPlugin

jhoaraupp · 2026-07-01T13:53:39Z

Description

PRE-3472: Fix error 500 when user proceed to return_cancel action on scalapay checkout

Motivation:

Related issue(s): Closes #

Type of Change

🐛 Bug fix (non-breaking change that fixes an issue) [x]
✨ New feature (non-breaking change that adds functionality) [ ]
💥 Breaking change (fix or feature that causes existing functionality to change and that could impact other libs) [ ]
🔧 Refactor (no functional changes, code improvement only) [ ]
📦 Dependency update [ ]
🔒 Security fix [ ]
📝 Documentation update [ ]

Checklist

Code Quality

Code is linted and formatted
No unnecessary commented-out code or debug logs
No hardcoded values (use env variables or config)

Testing

Unit tests added / updated

Security & Ops

No sensitive data or secrets introduced
Logging and error handling are appropriate

Copilot

Pull request overview

This PR aims to prevent a 500 error during Scalapay “return_cancel” by making payment state transitions idempotent (checking can() before apply()), so already-transitioned payments don’t trigger invalid-transition exceptions.

Changes:

Refactor payment status → transition mapping to compute a $transition first in both handlers.
Guard state-machine transitions with StateMachineInterface::can() before calling apply().
Add warning log + early return in the status handler when a transition can’t be applied.

1. What's Good

Prevents invalid state-machine transition exceptions by using can() checks before apply().
Adds structured context to the Status handler log (payment id, current state, transition, status), which helps diagnose edge cases.
Keeps the status→transition mapping centralized and easy to scan via match.

2. Summary table

Dimension	Rating
Security	✅ Fine
Correctness	⚠️ Medium (forced status may persist even when transition is skipped)
Performance	✅ Fine
Maintainability	⚠️ Low (inconsistent observability between handlers)

3. Closing one-liner

Address the forced-status persistence edge case (and improve observability when transitions are skipped) before merge.

4. Individual findings

Heading: Correctness ⚠️ Medium
Subtitle (bold): Forced status can persist even when transition is skipped (StatusPaymentRequestHandler.php:109)
Code block:

if (!$this->stateMachine->can($payment, PaymentTransitions::GRAPH, $transition)) {
    $this->logger->warning('[PayPlug] Cannot apply payment transition, payment is already in an incompatible state.', [
        'payment_id' => $payment->getId(),
        'current_state' => $payment->getState(),
        'transition' => $transition,
        'status' => $payment->getDetails()['status'] ?? '',
    ]);

    return;
}

Explanation paragraph: With forced status coming from the status query parameter, this early return prevents the exception (good), but it can also allow an overwritten details['status'] (e.g. canceled) to be persisted even though the Payment state remains completed. That creates a “details vs state” inconsistency that can confuse later logic that relies on details['status'].
Fix: Ensure forced status is only persisted when the transition is actually applied (e.g. move the forced-status write behind the can() check, or have updatePaymentState() return whether it applied and revert/skip the forced status when it didn’t).

Heading: Operational implications ⚠️ Low
Subtitle (bold): Log message is misleading when can() is false for idempotent calls (StatusPaymentRequestHandler.php:110)
Code block:

$this->logger->warning('[PayPlug] Cannot apply payment transition, payment is already in an incompatible state.', [

Explanation paragraph: can() can be false both for truly incompatible states and for “already applied” / idempotent scenarios; the current message only mentions incompatibility, which can cause unnecessary alerting/confusion.
Fix: Adjust the message to cover both cases.

$this->logger->warning('[PayPlug] Cannot apply payment transition (already applied or incompatible with current state).', [

Heading: Operational implications ⚠️ Low
Subtitle (bold): Skipped transitions become non-observable in Notify handler (NotifyPaymentRequestHandler.php:99)
Code block:

if ($this->stateMachine->can($payment, PaymentTransitions::GRAPH, $transition)) {
    $this->stateMachine->apply($payment, PaymentTransitions::GRAPH, $transition);
}

Explanation paragraph: This avoids exceptions (good), but it also removes the previous signal (exception → failed PaymentRequest) when transitions can’t be applied for unexpected reasons. Without logging, real data/config issues may go unnoticed while requests still complete.
Fix: Log when can() is false (inject LoggerInterface and log payment id/state/transition/status when skipping).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/Command/Handler/StatusPaymentRequestHandler.php	Adds `can()` guard + warning log around payment state transitions to avoid invalid-transition 500s.
src/Command/Handler/NotifyPaymentRequestHandler.php	Adds `can()` guard around payment state transitions to make notification handling idempotent.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

adumont-payplug · 2026-07-02T09:51:57Z

-            PayPlugApiClientInterface::FAILED => $this->stateMachine
-                ->apply($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_FAIL),
-            default => throw new \LogicException(sprintf('Unknown payment status "%s".', $payment->getDetails()['status'] ?? '')), // @phpstan-ignore-line - getDetails() return mixed
+        $transition = match ($payment->getDetails()['status'] ?? '') { // @phpstan-ignore-line - getDetails() return mixed


Maybe we could do a TransitionHelper with this to refacto those two files with this big duplica

Copilot

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.

+        $transition = match ($payment->getDetails()['status'] ?? '') { // @phpstan-ignore-line - getDetails() return mixed
+            PayPlugApiClientInterface::STATUS_ABORTED, PayPlugApiClientInterface::STATUS_CANCELED, PayPlugApiClientInterface::STATUS_CANCELED_BY_ONEY => PaymentTransitions::TRANSITION_CANCEL,
+            PayPlugApiClientInterface::STATUS_AUTHORIZED => PaymentTransitions::TRANSITION_AUTHORIZE,
+            PayPlugApiClientInterface::STATUS_CAPTURED => PaymentTransitions::TRANSITION_COMPLETE,
+            PayPlugApiClientInterface::FAILED => PaymentTransitions::TRANSITION_FAIL,
+            default => null,
        };


+        $transition = match ($payment->getDetails()['status'] ?? '') { // @phpstan-ignore-line - getDetails() return mixed
+            PayPlugApiClientInterface::STATUS_ABORTED, PayPlugApiClientInterface::STATUS_CANCELED, PayPlugApiClientInterface::STATUS_CANCELED_BY_ONEY => PaymentTransitions::TRANSITION_CANCEL,
+            PayPlugApiClientInterface::STATUS_AUTHORIZED => PaymentTransitions::TRANSITION_AUTHORIZE,
+            PayPlugApiClientInterface::STATUS_CAPTURED => PaymentTransitions::TRANSITION_COMPLETE,
+            PayPlugApiClientInterface::FAILED => PaymentTransitions::TRANSITION_FAIL,
+            default => null,
        };


+    private function updatePaymentState(PaymentInterface $payment): bool
    {
-        match ($payment->getDetails()['status'] ?? '') {
-            PayPlugApiClientInterface::STATUS_ABORTED, PayPlugApiClientInterface::STATUS_CANCELED, PayPlugApiClientInterface::STATUS_CANCELED_BY_ONEY => $this->stateMachine
-                ->apply($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_CANCEL),
-            PayPlugApiClientInterface::STATUS_AUTHORIZED => $this->stateMachine
-                ->apply($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_AUTHORIZE),
-            PayPlugApiClientInterface::STATUS_CAPTURED => $this->stateMachine
-                ->apply($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_COMPLETE),
-            PayPlugApiClientInterface::FAILED => $this->stateMachine
-                ->apply($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_FAIL),
-            default => throw new \LogicException(sprintf('Unknown payment status "%s".', $payment->getDetails()['status'] ?? '')), // @phpstan-ignore-line - getDetails() return mixed
+        $transition = match ($payment->getDetails()['status'] ?? '') { // @phpstan-ignore-line - getDetails() return mixed


    private function updatePaymentState(PaymentInterface $payment): void
    {
-        match ($payment->getDetails()['status'] ?? '') {
-            PayPlugApiClientInterface::STATUS_ABORTED, PayPlugApiClientInterface::STATUS_CANCELED, PayPlugApiClientInterface::STATUS_CANCELED_BY_ONEY => $this->stateMachine
-                ->apply($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_CANCEL),
-            PayPlugApiClientInterface::STATUS_AUTHORIZED => $this->stateMachine
-                ->apply($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_AUTHORIZE),
-            PayPlugApiClientInterface::STATUS_CAPTURED => $this->stateMachine
-                ->apply($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_COMPLETE),
-            PayPlugApiClientInterface::FAILED => $this->stateMachine
-                ->apply($payment, PaymentTransitions::GRAPH, PaymentTransitions::TRANSITION_FAIL),
-            default => throw new \LogicException(sprintf('Unknown payment status "%s".', $payment->getDetails()['status'] ?? '')), // @phpstan-ignore-line - getDetails() return mixed
+        $transition = match ($payment->getDetails()['status'] ?? '') { // @phpstan-ignore-line - getDetails() return mixed


Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

+        self::assertFalse($this->applier->apply($payment));
+    }
+}


+        $transition = match ($payment->getDetails()['status'] ?? '') { // @phpstan-ignore-line - getDetails() return mixed
+            PayPlugApiClientInterface::STATUS_ABORTED, PayPlugApiClientInterface::STATUS_CANCELED, PayPlugApiClientInterface::STATUS_CANCELED_BY_ONEY => PaymentTransitions::TRANSITION_CANCEL,
+            PayPlugApiClientInterface::STATUS_AUTHORIZED => PaymentTransitions::TRANSITION_AUTHORIZE,
+            PayPlugApiClientInterface::STATUS_CAPTURED => PaymentTransitions::TRANSITION_COMPLETE,
+            PayPlugApiClientInterface::FAILED => PaymentTransitions::TRANSITION_FAIL,
+            default => null,
+        };
+
+        if (null === $transition) {
+            $this->logger->warning('[PayPlug] Cannot apply payment transition: unknown status.', [
+                'sylius_payment_id' => $payment->getId(),
+                'payplug_payment_id' => $payment->getDetails()['payment_id'] ?? null,
+                'status' => $payment->getDetails()['status'] ?? '',
+            ]);
+
+            return false;
+        }
+
+        if (!$this->stateMachine->can($payment, PaymentTransitions::GRAPH, $transition)) {
+            $this->logger->warning('[PayPlug] Cannot apply payment transition (already applied or incompatible with current state).', [
+                'sylius_payment_id' => $payment->getId(),
+                'payplug_payment_id' => $payment->getDetails()['payment_id'] ?? null,
+                'current_state' => $payment->getState(),
+                'transition' => $transition,
+                'status' => $payment->getDetails()['status'] ?? '',


…scalapay checkout

Copilot AI review requested due to automatic review settings July 1, 2026 13:53

Copilot started reviewing on behalf of jhoaraupp July 1, 2026 13:54 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread src/Command/Handler/StatusPaymentRequestHandler.php Outdated

Comment thread src/Command/Handler/StatusPaymentRequestHandler.php Outdated

Comment thread src/Command/Handler/NotifyPaymentRequestHandler.php Outdated

jhoaraupp requested review from adumont-payplug and ilajili July 1, 2026 13:58

jhoaraupp force-pushed the fix/PRE-3472 branch from 0ef6ae8 to 9528f7e Compare July 1, 2026 14:03

jhoaraupp requested a review from Copilot July 1, 2026 14:03

Copilot started reviewing on behalf of jhoaraupp July 1, 2026 14:04 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread src/Command/Handler/StatusPaymentRequestHandler.php Outdated

Comment thread src/Command/Handler/StatusPaymentRequestHandler.php Outdated

Comment thread src/Command/Handler/NotifyPaymentRequestHandler.php Outdated

jhoaraupp force-pushed the fix/PRE-3472 branch from 9528f7e to ccd1f5f Compare July 1, 2026 14:11

jhoaraupp requested a review from Copilot July 1, 2026 14:12

Copilot started reviewing on behalf of jhoaraupp July 1, 2026 14:12 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

adumont-payplug reviewed Jul 2, 2026

View reviewed changes

jhoaraupp force-pushed the fix/PRE-3472 branch from ccd1f5f to 9781aba Compare July 2, 2026 12:02

Copilot AI review requested due to automatic review settings July 2, 2026 15:45

jhoaraupp force-pushed the fix/PRE-3472 branch from 9781aba to f310234 Compare July 2, 2026 15:45

Copilot started reviewing on behalf of jhoaraupp July 2, 2026 15:45 View session

Copilot AI reviewed Jul 2, 2026

View reviewed changes

jhoaraupp force-pushed the fix/PRE-3472 branch from f310234 to 0b1db42 Compare July 3, 2026 08:48

jhoaraupp requested a review from Copilot July 3, 2026 08:48

Copilot started reviewing on behalf of jhoaraupp July 3, 2026 08:49 View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

PRE-3472: Fix error 500 when user proceed to return_cancel action on …

9e08faf

…scalapay checkout

jhoaraupp force-pushed the fix/PRE-3472 branch from 0b1db42 to 9e08faf Compare July 3, 2026 09:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

PRE-3472: Fix error 500 when user proceed to return_cancel action on scalapay checkout#304

PRE-3472: Fix error 500 when user proceed to return_cancel action on scalapay checkout#304
jhoaraupp wants to merge 1 commit into
release/2.2.0from
fix/PRE-3472

jhoaraupp commented Jul 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

adumont-payplug Jul 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jhoaraupp commented Jul 1, 2026

Description

Type of Change

Checklist

Code Quality

Testing

Security & Ops

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

1. What's Good

2. Summary table

3. Closing one-liner

4. Individual findings

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

adumont-payplug Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants