Hi Dan,
On Wed, Jan 6, 2016 at 10:12 AM, Dan Ackroyd <dan...@basereality.com> wrote:
>
>
> // If FileException, NetworkException are thrown inside 'foo',
> // they are automatically
> // caught and execution continues to call bar()
> foo() suppress FileException, NetworkException;
> bar();
>
>
> That would allow people to ignore specific exceptions without having a
> huge amount of boilerplate code that makes the actual code illegible.
>
I like this suggestion a lot, it keeps the expected suppressions with the
call that will be suppressing.
> Possibly a more powerful technique would be needed, e.g.being able to
> specific an exception handler on a line by line basis.*
>
>
> // Execute function foo and if any exception is
> // raised, call the callable 'fooCallException'
> foo() raise 'fooRaiseException';
> bar();
>
> public function fooRaiseException(\Throwable $t)
> {
> if ($t instanceof FileException ||
> $t instanceof NetworkException) {
> // This is not an error in this situation, execution can continue.
> return;
> }
>
> // Other exceptions are not acceptable.
> // Propagation of the exception should continue as normal.
> throw $t
> }
>
>
I'm not sure I like this, because of a few things, it puts the suppressed
exceptions in a separate location from the call getting suppressed. Also,
having the function name as a string will make it harder to find in an IDE
since go to definition won't work (and since functions aren't first class
citizens as in javascript, we can't pass a raw name AFAIK) and admittedly
this reasoning doesn't count, IDE makers can update their IDE to have go to
definition work on strings after a raise keyword (however, this will get
complicated if ANY callable works, IE ['class','method'] and
'class::method'). Its basically moving the boilerplate you didn't like into
a function not connected to the call and allows accidental control flow
changes. Think if I do "foo() raises 'fooRaiseException'" everywhere I call
foo, but then in one place I need to suppress FileException and somewhere
else I don't want to. Someone comes along and changes the fooRaiseException
function and now the place where we need to know about FileException we no
longer do. This is caused by 2 things: the "handler" for the exceptions
isn't tied to the location where the exceptions happen, and "Find Usages"
in an IDE will not find strings referencing the function (especially if
built programmatically), again this second one is not valid because we
shouldn't make decisions based on what IDEs support but what's best for the
language. However I think its important to know the number of changes which
will be required in IDEs to make the feature usable for the average
developer (especially if someone from PHPStorm, NetBeans or ZendStudio came
along and said it would be impossible to support a specific new feature in
a usable way).
The benefits of doing it like this are:
>
> * Any unexpected exceptions are not silenced. This avoids the problem
> of all errors being silenced when the programmer thought only a single
> one would be suppressed.
>
> * It would allow users to debug their code. Even if an error is
> suppressed by the raise callable for a line of code, it would still be
> userland code that can be stepped through with a debugger, and so you
> can see the exception inside the 'raise' function.
>
> * It leaves the code in a readable state, which having lots of
> try/catch blocks does not.
>
> * It allows separate modules to have their own way of handling
> errors, whereas set_error_handler is a per application setting.
>
And what is the benefit of having either of these options being part of the
language (IIUC, this would mean changes to the parser, lexer, compiler and
run time) when it could be implemented in userland?
function tryAndSuppress(callable $func, array $args = [], array $suppress =
[])
{
try {
$ret = call_user_func_array($func, $args);
} catch (Throwable $e) {
foreach ($suppress as $type) {
if ($e instanceof $type) {
return;
}
}
throw $e;
}
return $ret;
}
$fh = tryAndSuppress('fopen', ['/path/to/some/file.ext', 'r'],
['FileNotFoundException']);
>
> As I said at the start, I think we need to have more powerful error
> handling in place before deprecating the silence operator. If anyone
> wants to progress this RFC, what I think needs to happen is:
>
> * Work on something like the above 'raise' or 'suppress' exceptions so
> that there is an alternative to the internal warnings.
>
> * Work on a plan on how to migrate all internal errors/warnings to
> individual exceptions. The current way of using set_error_handler to
> convert warnings/errors into generic exceptions isn't good enough.
> People need file operations to raise a more specific 'FileException',
> to allow catching of individual types of error. I don't think it would
> be feasible to just have a massive BC break on this...so a cunning
> migration plan would be needed.
>
Agree with this 100%. While I think errors suck and should all be turned
into exceptions, I also think the exceptions used need to be meaningful.
~ Ryan
