Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Nicer ApiErrorException::__toString() #1562

Merged
merged 3 commits into from
Aug 29, 2023
Merged

Nicer ApiErrorException::__toString() #1562

merged 3 commits into from
Aug 29, 2023

Conversation

richardm-stripe
Copy link
Contributor

@richardm-stripe richardm-stripe commented Aug 28, 2023 •
edited
Loading

Fixes #1561

Changes ApiErrorException::__toString() to have more useful information (a stack trace). This should be a safe change: __toString() is for human consumption and not intended to be consumed programatically.

Before this change:

(Status 200) (Request req_test) message

After this change:

Error sending request to Stripe: (Status 200) (Request req_test) message
Mock_ApiErrorException_21f57d2b: message in /Users/richardm/stripe/stripe-php/lib/Exception/ApiErrorException.php:38
Stack trace:
#0 /Users/richardm/stripe/stripe-php/tests/Stripe/Exception/ApiErrorExceptionTest.php(17): Stripe\Exception\ApiErrorException::factory('message', 200, '{"error": {"cod...', Array, Array, 'some_code')
#1 /Users/richardm/stripe/stripe-php/tests/Stripe/Exception/ApiErrorExceptionTest.php(45): Stripe\Exception\ApiErrorExceptionTest->createFixture()
#2 /Users/richardm/stripe/stripe-php/vendor/phpunit/phpunit/src/Framework/TestCase.php(1608): Stripe\Exception\ApiErrorExceptionTest->testToString()
#3 /Users/richardm/stripe/stripe-php/vendor/phpunit/phpunit/src/Framework/TestCase.php(1214): PHPUnit\Framework\TestCase->runTest()
#4 /Users/richardm/stripe/stripe-php/vendor/phpunit/phpunit/src/Framework/TestResult.php(728): PHPUnit\Framework\TestCase->runBare()
#5 /Users/richardm/stripe/stripe-php/vendor/phpunit/phpunit/src/Framework/TestCase.php(964): PHPUnit\Framework\TestResult->run(Object(Stripe\Exception\ApiErrorExceptionTest))
#6 /Users/richardm/stripe/stripe-php/vendor/phpunit/phpunit/src/Framework/TestSuite.php(684): PHPUnit\Framework\TestCase->run(Object(PHPUnit\Framework\TestResult))
#7 /Users/richardm/stripe/stripe-php/vendor/phpunit/phpunit/src/TextUI/TestRunner.php(653): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
#8 /Users/richardm/stripe/stripe-php/vendor/phpunit/phpunit/src/TextUI/Command.php(144): PHPUnit\TextUI\TestRunner->run(Object(PHPUnit\Framework\TestSuite), Array, Array, true)
#9 /Users/richardm/stripe/stripe-php/vendor/phpunit/phpunit/src/TextUI/Command.php(97): PHPUnit\TextUI\Command->run(Array, true)
#10 /Users/richardm/stripe/stripe-php/vendor/phpunit/phpunit/phpunit(98): PHPUnit\TextUI\Command::main()
#11 /Users/richardm/stripe/stripe-php/vendor/bin/phpunit(123): include('/Users/richardm...')
#12 {main}

@richardm-stripe richardm-stripe requested review from a team and anniel-stripe and removed request for a team August 28, 2023 18:32
@richardm-stripe richardm-stripe merged commit 18d6862 into master Aug 29, 2023
Playgirlkaybrazyworld referenced this pull request Aug 29, 2023
@richardm-stripe @pakrym-stripe
Change github workflow (#1460)
d13503a 
* Change github workflow

* Update Makefile

* Update tests/Stripe/GeneratedExamplesTest.php

* Update Makefile

* Yaml sucks

---------

Co-authored-by: pakrym-stripe <[email protected]>
@richardm-stripe
Copy link
Contributor Author

Released in https://github.com/stripe/stripe-php/releases/tag/v12.1.0

@remi-stripe remi-stripe deleted the richardm-nicer-exception-tostring branch September 28, 2023 23:02
@adaviloper
Copy link

I'm unsure if this change is why, but after upgrading from a version before this release, my Stripe exceptions are now seemingly being split by the new line character being added after including the entire stack trace.

I think this is breaking my logger since it's no longer receiving the format it's expecting.

This is more or less a shot in the dark, but does this at all make sense as a side-effect here or should I start looking into other solutions?

@adaviloper
Copy link

image

This is what I've been getting now after upgrading along with the rest of the stack trace which I've cropped out. You can see that the error being thrown is from the CardException class; not shown is that the reason is for an incorrect security code on the card.

It seems that at some point the way the exceptions are coerced into strings is including the entire stack trace and affecting the way DataDog is interpretting the error message.

@prathmesh-stripe
Copy link
Contributor

@adaviloper What version of dogstatd are you currently using? And what does your datadog agent configuration file?

@adaviloper
Copy link

@prathmesh-stripe We're on 7.61.0. Are there any keys that are of particular interest? I'm not able to post the entire file.

@prathmesh-stripe
Copy link
Contributor

prathmesh-stripe commented Feb 24, 2025 •
edited
Loading

I'm interested to know if there is any custom message formatting in the configuration file that I need to be aware of before I try to reproduce this error.
Ideally create an issue with as much detail as possible.

@adaviloper
Copy link

@prathmesh-stripe Appreciate the information. I'll go ahead and create an issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anniel-stripe anniel-stripe anniel-stripe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provide option to retain full stack traces in ApiErrorException
4 participants
@richardm-stripe @adaviloper @prathmesh-stripe @anniel-stripe