Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Get rid of error suppression #2210

Merged

Conversation

weierophinney
Copy link
Member

  • Thanks to Benoît Durand)
  • grep -r "@" library/Zend/ | grep -v "*" | grep -v "// @" used to
    identify suppressions
  • Used ErrorHandler to remove suppressions

- Thanks to intiilapa (Benoit Durand)
- grep -r "@" library/Zend/ | grep -v "*" | grep -v "// @" used to
  identify suppressions
- Used ErrorHandler to remove suppressions
@travisbot
Copy link

This pull request fails (merged 76e0592 into 3274889).

- trailing whitespace
@travisbot
Copy link

This pull request passes (merged a9eb6cc into 3274889).

@Maks3w
Copy link
Member

Maks3w commented Aug 21, 2012

The static review looks good for me. However I'll let this open for review for someone more

@marc-mabe
Copy link
Member

There are some components affected that doesn't depend on Zend\Stdlib before.
Is it practical adding this dependency only for ErrorHandler usage or should a native "set|reset_error_handler" used on such cases instead?

@marc-mabe
Copy link
Member

ErrorHandler::stop(); returns an instance of ErrorException or NULL so it can directly used as previous exception argument on throw.

@marc-mabe
Copy link
Member

I found 2 places of error_get_last(). This should be replaced with the return value of ErrorHandler::stop()

@weierophinney
Copy link
Member Author

@marc-mabe One of the places using error_get_last() requires its use due to a php limitation (I discovered that during testing yesterday) ; I'll look for the other locations however.

@weierophinney
Copy link
Member Author

ErrorHandler is simpler to implement and more consistent with usage across the framework; additionally, this is really simply completing an effort I thought I'd already accomplished a couple months ago. (It doesn't help that some new code crept in using error suppression, either.)

@weierophinney
Copy link
Member Author

@marc-mabe re: ErrorHandler::stop() + ErrorException -- I'll see if there are places that would be useful and update. Thanks.

- Removed two cases where error_get_last() was being used in Cache
  component; instead, captures the return from ErrorHandler::stop() and
  calls its getMessage() method.
@weierophinney
Copy link
Member Author

@Maks3w I think all of the questions @marc-mabe raised are answered at this point.

ErrorHandler::start(E_WARNING);
$fileHandle = fopen($this->filename, 'r');
ErrorHandler::stop();
if (false === $fileHandle) {
throw new Exception\UnexpectedValueException("Cannot open '$this->filename' for reading");
Copy link
Member

Choose a reason for hiding this comment

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

    //...
    $error = ErrorHandler::stop();
    if (false === $fileHandle) {
        throw new Exception\UnexpectedValueException("Cannot open '$this->filename' for reading", 0, $error); // previous error
    //...

weierophinney added a commit to zendframework/zend-authentication that referenced this pull request May 14, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
weierophinney pushed a commit to zendframework/zend-authentication that referenced this pull request May 14, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
weierophinney added a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
weierophinney pushed a commit to zendframework/zend-mail that referenced this pull request May 14, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
gianarb pushed a commit to zendframework/zend-serializer that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
weierophinney added a commit to zendframework/zend-text that referenced this pull request May 15, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
gianarb pushed a commit to zendframework/zend-text that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
gianarb pushed a commit to zendframework/zend-xmlrpc that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
gianarb pushed a commit to zendframework/zend-server that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
weierophinney added a commit to zendframework/zend-progressbar that referenced this pull request May 15, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
gianarb pushed a commit to zendframework/zend-progressbar that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
gianarb pushed a commit to zendframework/zend-filter that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
gianarb pushed a commit to zendframework/zend-ldap that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
gianarb pushed a commit to zendframework/zend-json that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
gianarb pushed a commit to zendframework/zend-validator that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
gianarb pushed a commit to zendframework/zend-file that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
weierophinney added a commit to zendframework/zend-log that referenced this pull request May 15, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
gianarb pushed a commit to zendframework/zend-log that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
weierophinney added a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
- Removed two cases where error_get_last() was being used in Cache
  component; instead, captures the return from ErrorHandler::stop() and
  calls its getMessage() method.
weierophinney added a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
weierophinney added a commit to zendframework/zend-view that referenced this pull request May 15, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
gianarb pushed a commit to zendframework/zend-view that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
weierophinney added a commit to zendframework/zend-navigation that referenced this pull request May 15, 2015
…s previous exception

- Per @mark-mabe
- Any place where an exception is throw immediately following an
  ErrorHandler::stop() call should pass the result of that call as the previous
  exception.
gianarb pushed a commit to zendframework/zend-navigation that referenced this pull request May 15, 2015
…y/hotfix/remove-suppression-operator

Get rid of error suppression
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants