Doc #74569 [Opn]: Return in a finally clause silently ignores an exception thrown in a try clause
| From: | requinix@php.net | Date: | Sun, 22 Nov 2020 22:25:01 +0000 |
| Subject: | Doc #74569 [Opn]: Return in a finally clause silently ignores an exception thrown in a try clause | ||
| References: | 1 | Groups: | php.doc.bugs |
| Request: | Send a blank email to doc-bugs+get-18147@lists.php.net to get a copy of this message | ||
Edit report at https://bugs.php.net/bug.php?id=74569&edit=1
ID: 74569
Updated by: requinix@php.net
Reported by: trianman at gmail dot com
Summary: Return in a finally clause silently ignores an
exception thrown in a try clause
Status: Open
Type: Documentation Problem
Package: Scripting Engine problem
Operating System: Any
PHP Version: 7.1.4
Block user comment: N
Private report: N
New Comment:
@anhlephuoc: The behavior you're seeing is because the "fatal error" from the
undefined function call is actually an exception that extends Error, not an exception that extends
Exception. You aren't catching it which means normally it would have been thrown - if not for
the fact that your finally block overrides that behavior and returns a value instead.
As of PHP 7, "catch (Exception $e)" does not catch every single exception that could be
thrown. Use the Throwable interface for that.
https://3v4l.org/6N4d9
Previous Comments:
------------------------------------------------------------------------
[2020-11-22 19:16:11] anhlephuoc at gmail dot com
This faulty behaviour of the return statement in the finally{} of the the try{} block extends beyond
the catchable error/exceptions.
1%0; // No warning or error.
unknown_function(); // not reported. Program not aborted
It took me days to locate a misspelled function, because no error is reported at all and program
would normally abort, now continue with undesirable behaviour.
Example code:
<?php
error_reporting(E_ALL|E_STRICT);
function do_test(): int {
try {
1/0; // a warning in php 7 - reported as documented
nonexistent_function(); // a fatal error - not reported - program continues
1%0; // a fatal error
return 0;
} catch (Exception $e) {
printf("CATCH\n");
return 1;
} finally {
printf("FINALLY\n");
return 2;
}
return 3;
}
printf("%d\n", do_test());
------------------------------------------------------------------------
[2017-05-16 21:16:02] danack@php.net
The Php Inspections (EA Extended) guy has added this as code smell to that tool.
You could consider using that tool and giving them some money for being so nice: https://www.indiegogo.com/projects/php-inspections-ea-extended-a-code-analyzer-security#/
------------------------------------------------------------------------
[2017-05-14 19:09:19] cmb@php.net
It seems to me this is simply a documentation issue.
------------------------------------------------------------------------
[2017-05-12 18:24:10] trianman at gmail dot com
2 danack@php.net
Sorry, I don't want to be mean. My English skills are not very good and it is sometimes hard to
express my thoughts in a clear way.
I've just realized that the finally statement is some-kind of antipattern for me. Because
finally is a shortcut for catching and re-throwing an \Exception class eg.
<?php
try {
throw new CustomException();
} catch (CustomException $ex) {
handleException();
} catch (\Exception $ex) {
doThingsInFinally();
throw $ex;
}
doThingsInFinally();
?>
Or with finally:
<?php
try {
throw new CustomException();
} catch (CustomException $ex) {
handleException();
} finally {
doThingsInFinally();
}
?>
Of cause if we have a return statement instead of a doTihingsInFinally() method, we will lose an
exception. But we doing it in a clear way. On the other hand finally statement hides out from
developer's eyes what actually is going.
So for my mind a little copy-paste is a lesser evil than an unclear behavior. For all other guys the
E_NOTICE will be enough. (:
------------------------------------------------------------------------
[2017-05-11 14:28:34] olavisau at gmail dot com
BC is broken with any solution. The fact that javascript behaves in the same way worries me.
It's probably very hard to implement. I agree with the E_NOTICE.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=74569
--
Edit this bug report at https://bugs.php.net/bug.php?id=74569&edit=1