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

Avoid PHP warnings due to lack of args in exception trace on PHP 7.4 #160

Merged
merged 1 commit into from
Jun 15, 2020

Conversation

clue
Copy link
Member

@clue clue commented Jun 13, 2020

It looks like some installations of PHP 7.4 do not report the "args" in the exception trace anymore. This can lead to reporting some PHP warnings (PHP Warning: Invalid argument supplied for foreach()) and/or rejected promises depending on the error handler used.

Accordingly, this didn't show up during unit tests because PHPUnit transforms these warnings into exceptions automatically which will cause the underlying promise to be rejected – which will not be reported by default (reactphp/promise#87). This can be best reproduced by running one of the examples with an invalid DNS server address and error reporting enabled. This should now report a promise rejection without any PHP warnings.

Builds on top of clue/reactphp-http-proxy#33 and #118.

@clue clue added this to the v1.3.0 milestone Jun 13, 2020
@ghost
Copy link

ghost commented Jun 13, 2020

There is a new INI setting called zend.exception_ignore_args in PHP 7.4:

zend.exception_ignore_args is a new INI directive for including or excluding arguments from stack traces generated from exceptions.

https://www.php.net/manual/en/migration74.other-changes.php#migration74.other-changes.ini
https://3v4l.org/CS8S9

@clue
Copy link
Member Author

clue commented Jun 14, 2020

There is a new INI setting called zend.exception_ignore_args in PHP 7.4 […]

Thanks for the find @CharlotteDunois, that's the configuration I was missing! It's my understanding this confirms this suggested changeset makes sense as-is 👍

@clue clue requested review from jsor and WyriHaximus June 14, 2020 16:10
@jsor jsor merged commit 9943b55 into reactphp:master Jun 15, 2020
@clue clue deleted the trace branch June 15, 2020 08:03
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.

3 participants