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

[ErrorHandler][FrameworkBundle] better error messages in failing tests #36003

Merged
merged 1 commit into from
Mar 16, 2020
Merged

[ErrorHandler][FrameworkBundle] better error messages in failing tests #36003

merged 1 commit into from
Mar 16, 2020

Conversation

guillbdx
Copy link

@guillbdx guillbdx commented Mar 9, 2020

Q A
Branch? master for features
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #32752
License MIT
Doc PR

Purpose of this PR is to enhance tests by giving a way to report an exception that occured during the processing of the request.

The ErrorHandler will add an X-Debug-Exception, and the assertThat() method of WebTestCase will throw an exception if this header exists and status code is 5xx.

In practice, this adds the "Caused by" section in this example:

Time: 374 ms, Memory: 20.00 MB

There was 1 failure:

1) App\Tests\Controller\HomeControllerTest::testC
Failed asserting that the Response has header "Content-Type" with value "application/json".

/srv/symfony/src/Symfony/Bundle/FrameworkBundle/Test/BrowserKitAssertionsTrait.php:132
/srv/symfony/src/Symfony/Bundle/FrameworkBundle/Test/BrowserKitAssertionsTrait.php:66
/srv/blog/tests/Controller/HomeControllerTest.php:29

Caused by
Exception: This a test exception. in /the/file.php:139
Stack trace:
[...]

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 9, 2020
@guillbdx guillbdx changed the title Feature/32752 better error messages in failing tests [ErrorHandler][FrameworkBundle] better error messages in failing tests Mar 9, 2020
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! No more blind errors when using this testing API \o/

Copy link
Member

@yceruto yceruto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments, thank you!

@fabpot
Copy link
Member

fabpot commented Mar 16, 2020

Thank you @guillbdx.

@sstok
Copy link
Contributor

sstok commented Mar 22, 2020

Nice! This was the one thing missing from the new testing assertions. Thanks for working on this.

@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
@eng1neer
Copy link

I'm getting an nginx error caused by a too large X-Debug-Exception header (upstream sent too big header while reading response header from upstream). This happens with a moderately large exception text (few kilobytes) and default nginx fastcgi buffer size settings. Is there any way to disable the X-Debug-Exception header population without disabling Symfony's debug mode altogether?

@derrabus
Copy link
Member

Hello @eng1neer and thank you for reaching out.

This tracker is meant for tracking bugs and feature requests. If you are looking for support, you'll get faster responses by using one of our official support channels:

See https://symfony.com/support for more support options.

@eng1neer
Copy link

Hey, @derrabus thanks for taking the time to flag my message as off-topic.

I would argue that this is actually a bug. Stuffing data of an arbitrary length into HTTP headers is a bad idea. Most of the web servers impose a limit on the amount of data that can be transmitted in HTTP headers and this limit is not too big (4-8KB for Apache or Nginx). You can easily exceed it with a few kilobytes of text in the exception message which leads to an unexpected error page generated by the webserver.

There should be other, more appropriate ways to pass exception information in tests that will not intervene with the usual development workflow.

@derrabus
Copy link
Member

@eng1neer All right then, please file a bug report.

PhilETaylor pushed a commit to PhilETaylor/symfony that referenced this pull request Sep 6, 2023
…ssages in failing tests (guillbdx)

This PR was squashed before being merged into the 5.1-dev branch.

Discussion
----------

[ErrorHandler][FrameworkBundle] better error messages in failing tests

| Q             | A
| ------------- | ---
| Branch?       | master for features
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix symfony#32752
| License       | MIT
| Doc PR        |

Purpose of this PR is to enhance tests by giving a way to report an exception that occured during the processing of the request.

The ErrorHandler will add an X-Debug-Exception, and the assertThat() method of WebTestCase will throw an exception if this header exists and status code is 5xx.

In practice, this adds the "Caused by" section in this example:

```
Time: 374 ms, Memory: 20.00 MB

There was 1 failure:

1) App\Tests\Controller\HomeControllerTest::testC
Failed asserting that the Response has header "Content-Type" with value "application/json".

/srv/symfony/src/Symfony/Bundle/FrameworkBundle/Test/BrowserKitAssertionsTrait.php:132
/srv/symfony/src/Symfony/Bundle/FrameworkBundle/Test/BrowserKitAssertionsTrait.php:66
/srv/blog/tests/Controller/HomeControllerTest.php:29

Caused by
Exception: This a test exception. in /the/file.php:139
Stack trace:
[...]
```

Commits
-------

0da9469 [ErrorHandler][FrameworkBundle] better error messages in failing tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better error messages in failing tests
8 participants