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

Convert E_ERROR to thrown Error in extensions #1942

Merged
merged 9 commits into from Jul 5, 2016

Conversation

trowski
Copy link
Member

@trowski trowski commented Jun 11, 2016

This PR converts most occurrences in extensions of E_ERROR with thrown Error objects. Not all fatal errors were converted to exceptions. Those dealing with memory allocation or other unrecoverable errors have remained fatal. I also did not covert any fatal errors in the Soap extension. The extension appears to rely heavily on bailout behavior, so fatals could not easily be converted to thrown exceptions. Perhaps someone with more experience with the Soap extension would be able to do so.

This PR supersedes #1388.


RFC: https://wiki.php.net/rfc/throw_error_in_extensions

@hoshsadiq
Copy link

Perhaps a good idea to add tests to ensure Throwable and/or Error are caught

@trowski trowski force-pushed the throw-error-in-extensions branch 3 times, most recently from 1001fb9 to 7d53864 Compare June 18, 2016 14:44
@laruence laruence added the RFC label Jun 28, 2016
@@ -5633,7 +5635,7 @@ ZEND_METHOD(reflection_property, getValue)
return;
}
if (Z_TYPE(CE_STATIC_MEMBERS(intern->ce)[ref->prop.offset]) == IS_UNDEF) {
php_error_docref(NULL, E_ERROR, "Internal error: Could not find the property %s::%s", ZSTR_VAL(intern->ce->name), ZSTR_VAL(ref->prop.name));
zend_throw_error(NULL, "Internal error: Could not find the property %s::%s", ZSTR_VAL(intern->ce->name), ZSTR_VAL(ref->prop.name));
/* Bails out */
Copy link
Member

Choose a reason for hiding this comment

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

should recover and return

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, missed this one.

@php-pulls php-pulls merged commit e9832b5 into php:master Jul 5, 2016
@trowski trowski deleted the throw-error-in-extensions branch April 28, 2021 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants