Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

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

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
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 •
edited by nicolas-grekas
Loading

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
nicolas-grekas
nicolas-grekas reviewed Mar 12, 2020
View reviewed changes
src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md Outdated Show resolved Hide resolved
src/Symfony/Bundle/FrameworkBundle/Test/BrowserKitAssertionsTrait.php Outdated Show resolved Hide resolved
src/Symfony/Bundle/FrameworkBundle/Test/BrowserKitAssertionsTrait.php Outdated Show resolved Hide resolved
nicolas-grekas
nicolas-grekas reviewed Mar 13, 2020
View reviewed changes
src/Symfony/Component/ErrorHandler/CHANGELOG.md Outdated Show resolved Hide resolved
nicolas-grekas
nicolas-grekas approved these changes Mar 13, 2020
View reviewed changes
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/

@nicolas-grekas nicolas-grekas requested a review from yceruto March 13, 2020 11:37
@nicolas-grekas nicolas-grekas mentioned this pull request Mar 13, 2020
Closed
yceruto
yceruto reviewed Mar 13, 2020
View reviewed changes
src/Symfony/Component/ErrorHandler/CHANGELOG.md Outdated Show resolved Hide resolved
yceruto
yceruto reviewed Mar 13, 2020
View reviewed changes
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.

@fabpot fabpot merged commit 693cf11 into symfony:master Mar 16, 2020
@fabpot fabpot mentioned this pull request Mar 16, 2020
Closed
@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
Merged
@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.

@paxal paxal mentioned this pull request Nov 29, 2022
Closed
PhilETaylor pushed a commit to PhilETaylor/symfony that referenced this pull request Sep 6, 2023
@fabpot
feature symfony#36003 [ErrorHandler][FrameworkBundle] better error me…
541fff7 
…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
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@fabpot fabpot fabpot approved these changes

@yceruto yceruto yceruto approved these changes

@javiereguiluz javiereguiluz Awaiting requested review from javiereguiluz

Assignees
No one assigned
Labels
Feature Status: Reviewed
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

Better error messages in failing tests
8 participants
@guillbdx @fabpot @sstok @eng1neer @derrabus @nicolas-grekas @yceruto @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.