🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@kbond
Copy link
Member

@kbond kbond commented May 24, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR todo

Debugging exceptions in functional tests is difficult as you need to look at the logs to see which exception was thrown. Disabling catching of exceptions in the client would allow the exception to bubble up to phpunit and make it easier to see what exception was thrown.

@nicolas-grekas
Copy link
Member

new features should be submitted against 3.4

@kbond kbond changed the base branch from master to 3.4 May 24, 2017 15:07
class Client extends BaseClient
{
protected $kernel;
protected $catchExceptions = true;
Copy link
Member

Choose a reason for hiding this comment

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

should be private

* Sets whether to catch exceptions when the kernel is handling a request.
*
* @param bool $catchExceptions Whether to catch exceptions
*/
Copy link
Member

Choose a reason for hiding this comment

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

having a default value here does not very useful to me

@kbond
Copy link
Member Author

kbond commented Sep 26, 2017

Anything preventing this from making it into 3.4?

Copy link
Contributor

@ogizanagi ogizanagi left a comment

Choose a reason for hiding this comment

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

Missing CHANGELOG.md entry ? :)

* Returns whether client is catching kernel exceptions or not.
*
* @return bool
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this method really useful?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, I was just matching Symfony\Component\BrowserKit\Client::isFollowingRedirects()

Copy link
Member

Choose a reason for hiding this comment

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

let's remove it :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@kbond
Copy link
Member Author

kbond commented Sep 26, 2017

I added a CHANGELOG.md entry.

Let me know if you want me to remove Client::isCatchingExceptions()

@ogizanagi
Copy link
Contributor

Thank you @kbond.

@ogizanagi ogizanagi merged commit 4812e60 into symfony:3.4 Sep 26, 2017
ogizanagi added a commit that referenced this pull request Sep 26, 2017
…ons for Client (kbond)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Add ability to configure catching exceptions for Client

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | todo

Debugging exceptions in functional tests is difficult as you need to look at the logs to see which exception was thrown. Disabling catching of exceptions in the client would allow the exception to bubble up to phpunit  and make it easier to see what exception was thrown.

Commits
-------

4812e60 add ability to configure catching exceptions
This was referenced Oct 18, 2017
robfrawley added a commit to robfrawley/liip-imagine-bundle that referenced this pull request May 7, 2018
…anged

browser kit client to not catch exceptions to fix our test suite.

References:
  - https://symfony.com/blog/new-in-symfony-4-1-deprecated-the-bundle-notation
  - symfony/symfony#26085
  - symfony/symfony#22890

Signed-off-by: Rob Frawley 2nd <rmf@src.run>
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.

5 participants