Problem/Motivation

Stripe's ApiErrorException provides useful error codes (and their CardException provides decline codes). Currently Commerce Stripe's ErrorHelper both fails to log this information and makes it unavailable to custom debugging or logging code.

Proposed resolution

1. Append the result of ApiErrorException::getStripeCode() and CardException::getDeclineCode() to the message logged to the site log.
2. When ErrorHelper throws new exceptions in place of Stripe's ApiErrorExceptions, provide the original ApiErrorException as the "previous" parameter to the new exceptions. This makes it available to custom code catching the thrown exception, by calling $exception->getPrevious().

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork commerce_stripe-3258663

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jonathanshaw created an issue. See original summary.

jonathanshaw opened merge request !9

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Active » Needs review

I've also removed an elseif ($exception instance of ApiErrorException) that is unnecessary because the function isgnature guarantees that the supplied $exception is an ApiErrorException.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Title: Supply Stripe ApiErrorException as previous when rethrowing in ErrorHelper » Enhance logging of Stripe ApiErrorExceptions in ErrorHelper
Issue summary: View changes
shabana.navas’s picture

shabana.navas commented
Status: Needs review » Reviewed & tested by the community

Patch looks good and working. Though, we might need tests for this.

chandeepkhosa’s picture

chandeepkhosa commented

I can also confirm that the patches continue to apply smoothly for 1.0-rc10 & 1.0. It would be really great if we could look at getting this merging in soon :)

simgui8’s picture

simgui8 commented

I have been using this for months, still applies on today dev release.
Thanks @jonathanshaw

rszrama made their first commit to this issue’s fork.

rszrama’s picture

rszrama commented
Status: Reviewed & tested by the community » Fixed

I honestly have no clue what a merge train is or why GitLab always thinks I want one. This has been merged. 😅

Status: Fixed » Closed (fixed)

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