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.

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

Option to print previous exception #286

Open
oprypkhantc opened this issue Sep 12, 2023 · 1 comment
Open

Option to print previous exception #286

oprypkhantc opened this issue Sep 12, 2023 · 1 comment

Comments

@oprypkhantc
Copy link

Hey.

Currently there's no option for Collision to print the Throwable::getPrevious() exception alongside the main one, but it'd be nice to have one.

Our use case specifically is this: we have Laravel integration tests that make HTTP requests. When a HTTP code assertion fails due to an exception during the request, Laravel attempts to add some debugging information by adding this kind of text stack trace to the exception message (by modifying Exception::$message through reflection):

The following exception occurred during the last request:

TypeError: [redacted]: Return value must be of type float, null returned in [redacted]
Stack trace:
.. a lot of trace lines here
#84 /app/tests/TestCase.php(144): Illuminate\Foundation\Testing\TestCase->postJson()
.. a lot of trace lines here
#96 /app/vendor/phpunit/phpunit/src/TextUI/TestRunner.php(63): PHPUnit\Framework\TestSuite->run()
#97 /app/vendor/phpunit/phpunit/src/TextUI/Application.php(177): PHPUnit\TextUI\TestRunner->run()
#98 /app/vendor/phpunit/phpunit/phpunit(99): PHPUnit\TextUI\Application->run()
#99 {main}

----------------------------------------------------------------------------------

[redacted]: Return value must be of type float, null returned

  at tests/[redacted]
    290▕                        ],
    291▕                ])
  ➜ 292▕                        ->assertSuccessful()
    293▕                        ->assertJson([
    294▕                                'meta' => [
    295▕                                        'rules' => [


  Tests:    1 failed (1 assertions)
  Duration: 20.35s

Although this does work, it'd be much nicer to have the original exception rendered natively by Collision. Laravel could then set Exception::$previous instead of the Exception::$message and it'd be rendered something like this:

at tests/[redacted]
    290▕                        ],
    291▕                ])
  ➜ 292▕                        ->assertSuccessful()
    293▕                        ->assertJson([
    294▕                                'meta' => [
    295▕                                        'rules' => [

Caused by an exception at tests/[redacted]:103: Return value must be of type float, null returned

    101▕                // code
    102▕                // code
  ➜ 103▕                // code
    104▕                // code
    105▕                // code
    106▕                // code

  Tests:    1 failed (1 assertions)
  Duration: 20.35s

That would improve QoL by reducing the stack trace (from 100+ lines of vendor code to just a few lines that are actually important) and rendering it in an eye-friendly way.

Besides the need of a new option and some additions to the inspectors, this addition looks trivial to implement, so if this is something that can get merged, I can PR the changes.

@oprypkhantc
Copy link
Author

So it turns out it does work, just not at all how you expect it to. The "previous" exception is included in the trace, but this is how it currently looks:

  at [redacted]
    106▕                        // code
    107▕                        // code
    108▕                        // code
    109▕                        // code
  ➜ 110▕                        // code
    111▕                        // code
    112▕                        // code
    113▕                        // code
    114▕        }

  1   app/SomeFile:67
      NunoMaduro\Collision\Exceptions\TestException::("asdasd")
  2   app/SomeFile:67

A few things here:

  • instead of the actual exception class name, we get NunoMaduro\Collision\Exceptions\TestException
  • throw location is duplicated twice
  • only 1 usable trace line is included. If you run tests with SHELL_VERBOSITY=1, it does show the complete trace - but in that case it also includes 100 trace lines from vendor which clutters the output. I'd rather see the complete stack trace of userland code than 1 line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant