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] Never throw on warnings triggered by assert() and set assert.exception=1 in Debug::enable() #35645

Merged
merged 1 commit into from Feb 8, 2020

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

Because we don't use assert(), this is something we completely overlooked, but warnings triggered should not throw as there is already a dedicated exception mode when using assert().

This turns this exception mode to 1 in debug mode and logs the assert() warnings in prod.

@fabpot
Copy link
Member

fabpot commented Feb 8, 2020

Thank you @nicolas-grekas.

fabpot added a commit that referenced this pull request Feb 8, 2020
…() and set assert.exception=1 in Debug::enable() (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[ErrorHandler] Never throw on warnings triggered by assert() and set assert.exception=1 in Debug::enable()

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Because we don't use `assert()`, this is something we completely overlooked, but warnings triggered should not throw as there is already a dedicated exception mode when using `assert()`.

This turns this exception mode to 1 in debug mode and logs the assert() warnings in prod.

Commits
-------

f18ef6c [ErrorHandler] Never throw on warnings triggered by assert() and set assert.exception=1 in Debug::enable()
@fabpot fabpot merged commit f18ef6c into symfony:4.4 Feb 8, 2020
@nicolas-grekas nicolas-grekas deleted the eh-assert branch February 8, 2020 16:52
This was referenced Feb 29, 2020
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.

None yet

4 participants