Skip to content

Instantly share code, notes, and snippets.

@Danack

Danack/error.md Secret

Last active November 29, 2018 11:09
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
Star You must be signed in to star a gist
Save Danack/5ae0b1b1ce30a0d785dd to your computer and use it in GitHub Desktop.
error control improvements.

Introductory disclaimer - all exception names other than \Exception and \Throwable are placeholder names, with the exact names to be chosen later.

Improved error suppression operator

Foreword

The feedback I have received is that this RFC contains many words. Here is the TL:DR version first.

This RFC proposal:

  • Only affects error handlers set through set_error_handler.
  • It does not affect exceptions that have been thrown.
  • Adds a parameter when user set error_handlers are called. This new parameter is the exception class name that should be thrown if the error_handler chooses to throw an exception.
  • Adds a "suggested exception type" to every warning/notice/error that is triggered by PHP internal code with a default of \Exception. This "suggested exception type" is only passed to user defined error handlers when an error is triggered. It does not affect anything else.
  • Proposes allowing a new syntax to suppress warning/notice/errors when their "suggested exception type" matches any of the types listed.

The benefits of this RFC are:

  • Allows user to precisely control which errors get turned into exceptions, at a much finer grain of control than error_level reporting.
  • Allows fine grained collaboration between calling and called code; much finer than exceptions alone provide.
  • Even if you don't want errors to be converted to exceptions, the suggested exception type allows more precise logging of error conditions.
  • Provides those benefits with no BC breaks and without having to have the horrible discussion of whether certain errors/notices/warnings are superfluous.

Introduction

The error suppression operator serves a useful purpose. It allows calling code to collaborate with the code it is calling, and tell it "If an error occurs, and you're able to continue, please ignore the error". When used with a error handler, set through set_error_handler, that converts all unsuppressed errors to exceptions, it allows people using PHP to control on a line-by-line basis i) What errors are acceptable conditions, and execution should continue, ii) What errors are not acceptable conditions, and so an exception should be thrown.

Example - division by zero

The two functions below are identical, but have different semantic meaning

// Calculate a user's priority for a task algorithm is similar to
// the one used by the Slurm workload manager [2]
// Under-served users will have a value greater than 1.0
// Over-served users will have a value between 0.0 and 1.0.
function calculatePriority($sharesNorm, $usage) 
    return $sharesNorm / $usage;
}

// Calculate the price per unit of items that are sold as a multi-pack
function calculatePricePerUnit($totalPrice, $numberInPack) {
    return $totalPrice / $numberInPack;
}


$priority = calculatePriority(20, 0);
// $priority is now infinity, this is an acceptable result as it means that
// it is the highest possible priority.

// $multiPack has had its "number of units" erroneously set to zero.
$pricePerUnit = calculatePricePerUnit($multiPack->getPrice(), $multiPack->getNumberOfUnits());
// $pricePerUnit is now infinity, which is a gibberish result.

For one of those functions, having the result be infinity is meaningless, for the other it's perfectly fine.

By setting an error_handler that throws exceptions when unsuppressed errors are encountered, and then altering the 'calculatePriority' function to use the error control operator:

function calculatePriority($sharesNorm, $usage) 
    return @($sharesNorm / $usage);
}

Both functions now have the right behaviour. 'calculatePriority' will continue to generate infinity as the result when the 2nd parameter is zero, and 'calculatePricePerUnit' will now cause throw exception to be thrown if the 2nd parameter is zero.

Example - mkdir

Again, we have two functions that are identical, but have different semantic meaning:

// Create a data file in a new directory 
// $directoryName should be a unique directory name that
// does not currently exist
function createUniqueFile($directoryName, $text) {
    mkdir($directoryName, 0755, true);
    file_put_contents("foo.txt", $text);
}

// Update a data file in a directory that may already exist
function updateData($directoryName, $text) {
    mkdir($directoryName, 0755, true);
    file_put_contents("foo.txt", $text);
}

For the first function, the failure of mkdir to create a directory due to it already existing is an error condition that means continuing execution of the function is not possible.

For the second function, the failure of mkdir to create a directory is not a problem, and it is fine for execution to continue.

Again, by setting an error_handler that throws exceptions when unsuppressed errors are encountered, and then altering the function where it is desirable for execution to continue past the error condition to use the error operator:

function updateData($directoryName, $text) {
    @mkdir($directoryName, 0755, true);
    file_put_contents("foo.txt", $text);
}

Again, both functions now have the right behaviour. 'updateData' will work without any complaint when the directory already exists, and 'createUniqueFile' will cause an exception to be thrown by the set error_handler.

Downsides of current solution

The current error control operator has some very serious downsides when it is used.

Binary choice - all errors/warnings are suppressed or not.

The errors/warnings generated by internal code are either all suppressed or none are. This means that a programmer can accidentally suppress far more errors than they intend to. Take the case of setting some values in an array.

The programmer doesn't want to be warned when the array key doesn't already exist, as the behaviour of PHP is to set it to zero, which is the desired behaviour.

$foo = [];

$index = 2;
// The @ suppresses the "index does not exist" error but
// otherwise the operation continues
@$foo[$index]++; 

However what they might not realise is this also suppresses other errors for example the error when the key is not a valid value to be used as a key

$index = new StdClass;
// This operation fails completely as objects can't be used as keys
// but the @ completely suppresses this error also.
@$foo[$index]++; 

It should be possible for users to be able to suppress the "index does not exist" warning, without also suppressing the "wrong type for index" error.

Lack of semantic meaning

Because there is no exception type associated with errors/warnings it is not easy to know what would be an appropriate exception for internal errors. Although there are some attempts to convert internal errors to specific exception types[1], they rely on string matching to work and so are not 100% reliable.

This means that it is not possible to selectively catch errors/warnings that have been converted to exceptions by a user error handler, as the error handler can only reliably throw \Exceptions.

Proposal

This RFC proposes:

  • Associate to each notice/error generated by internal code a 'suggested exception type' that should be thrown, if the error handler thinks an exception should be thrown.

  • Make the error suppression operator more powerful by allowing users to suppress individual types of error.

These proposals should have no impact on existing code.

Details of proposal

The details of the implementation are below. Syntax choice has not finalised and may change. Please see the section on syntax choice before proposing alternatives.

Add 'suggested exception type' to error_handler callback

An additional parameter will be passed to any user-defined error handler. The parameter will be the exception type that should be thrown, if the error handler decides to throw an exception

bool handler (int $errno , string $errstr, string $errfile, int $errline, array $errcontext )

to

bool handler ( int $errno , string $errstr, string $errfile, int $errline , array $errcontext, string $exception_type )

If the error cannot be handled in the current context, an exception of type $exception_type SHOULD (not must) be thrown by the error handler.

Add suppress operator

Allows the caller to indicate to error_handlers that a particular 'suggested exception type' should be ignored and not throw an exception.

foo(1, 2, 3) @suppress('DivisionByZeroException', 'ArrayAccessException');

This should be read as "when calling foo() if an error/warning is raised, that would pass a 'suggested exception type' of either 'DivisionByZeroException' or 'ArrayAccessException' to a user error_handler, temporarily change the error_reporting level to be 0 when the error handler is called.

The suppress error operator is a language construct (not a function) that will take any number of strings as 'parameters'. Only strings that are resolvable at compile-time are supported i.e. quote enclosed strings and class names such as Foo::class.

When an error occurs, before the error handler is triggered, if the 'suggested exception type' matches one which is currently suppressed, the error level (as returned by error_reporting()) will be temporarily changed to 0. This is the same mechanism that the current error control operator uses.

Add exception type to trigger_error

Change signature of triggor_error to accept the a 'suggested exception type' that will be passed to any user error_handler called.

bool trigger_error( string $error_msg [, int $error_type = E_USER_NOTICE ] )

to

bool trigger_error( string $error_msg [, int $error_type = E_USER_NOTICE [, string $exception_type = 'Exception' ]] )

Syntax choice

While the syntax choice isn't final, other syntaxes will only be considered if they are technically superior. I realise that seeing new syntax usually gives a negative initial reaction. Before criticising the syntax choice, please imagine that you've been using it for 6 months and have become accustomed to it. In that scenario what deficiencies would it have?

The suggested syntax has several benefit over other choices. One alternative, which this RFC does not want to consider, is to have the suppress error operators before the code they are affecting, using a scope block, like this:

@suppress('BarException') {
    foo();
}

Comparing the two syntax choices shows two deficiencies with that alternative syntax

Error suppression inside function calls

With the proposed syntax it is reasonably acceptable. With the alternative 'scoped' syntax, it's harder to read

// This is reasonably clear.
foo(bar() @suppress('ArrayAccessException'));

// This is harder to read.
foo(suppress('ArrayAccessException') {bar()});

Example error handler implementation

An example of a new error handler userland implementation that throws an exception of the suggested type.

function errorHandler($errno, $errstr, $errfile, $errline, $suggested_exception_type)
{
    if (error_reporting() === 0) {
        // Error reporting has be silenced
        return true;
    }
    if ($errno === E_CORE_ERROR || $errno === E_ERROR) {
        // For these two types, PHP is shutting down anyway. Return false
        // to allow shutdown to continue
        return false;
    }

    throw new $suggested_exception_type($errstr);
}

Add exception type to throw to all internal code

To all errors, warnings and notices raised by internal code, an exception type to throw will be added. If no exception type is set, a default type of '\Exception' will be assumed. The default type of \Exception will apply to all extensions where specific exception types have not been added.

THESE 'SUGGESTED EXCEPTION TYPES' WILL HAVE NO DIRECT EFFECT. THEY ARE ONLY PASSED TO USER ERROR HANDLERS.

i.e. there will be no BC break for existing code.

Examples after refactoring

calculatePriority(20, 0) @suppress('DivisionByZeroException');
function createUniqueFile($directoryName, $text) {
    mkdir($directoryName, 0755, true) @suppress('Exception');
    file_put_contents("foo.txt", $text);
}

Out of scope

Deprecating existing error suppression operator.

Although the capabilities this RFC hopes to add to PHP may make the existing error control operator be redundant, the actual decision to either deprecate or remove the original error operator needs to be a separate decision, made at a later date.

Exact exceptions to be used

The exact exceptions to be associated with errors/warnings generated but internal code will be determined in a separate RFC. There are approximately 2700 errors/warnings/notices that need to be looked at. It is too much work to do as part of an RFC that might not pass.

User defined handlers

Although the syntax currently chosen could allow custom exception raising functions, they are out of scope of this RFC. However to show that the chosen syntax is compatible with allowing user defined error operator functions,


function loggingHandler(string $exceptionType) {
    //special magic goes here.
}

// Reigster
register_error_operator('log', 'loggingHandler');

foo() @log('FooException');

To re-iterate though, user defined error operators are not part of this RFC. The only error operators this RFC seek to add is the provided 'suppress' operation. Any user defined functions would need to be part of a separate RFC.

Feedback so far

People should just check for errors!!!

Writing code that checks for errors constantly sucks. i) People forget to check ii) It's illegible.

But worse than this it makes it hard to write code that is safe to use against future versions of functions. When an error condition is added to a function, where none could previously occur, no code that calls that function will be checking for errors. Realistically this means that adding error conditions that need to be checked to existing functions will result in errors occurring silently, and for user applications to continue to run in a silent error state.

We should use exceptions for everything!!!

As the introduction to this RFC tries to explain, there are some cases where it is borderline whether a particular condition is actually an error or an ignore-able condition. Although some of the errors/warnings that are generatd by internal code could be converted to excptions I do not belive there will ever be consensus to a solution that involves dramatically increasing the number of try/catch blocks that would be needed to write PHP code.

But even if there were consensus, exceptions are not as powerful as error suppression. Throwing an exception stops the execution of the current code, and execution resumes at a high block that where the exception is handled. Error suppression allows the calling code to determine whether particular errors should be converted into exceptions. That allows execution to continue from where the error occured, if the calling code says that it's okay to do so.

For example, a function that does a math operation:

function foo($p1, $p2, $p3) {
    $data['foo'] = $p1 / $p2;
    $data['bar'] = $p2 / $p3;
    $data['quux'] = $p1 + $p2 + $p3;
}

$bar = foo(5, 10, 0) @suppress('DivideByZeroException');

With only using try/catch, it is not possible for the execution of the function to continue and generate a result. If the calling code really wants $bar to contain the resulting value, because they have explicitly said that 'Divide by zero error' conditions are to be expected, then that can be done with error suppression. It can't be done with try/catch.

This seems like it could be misused!!!

Definitely. However the position of this RFC is that any feature that is going to give developers all of the power that they need to write good applications can by it's nature be used for purposes that are bad practice. Almost any feature that cannot be misused is by its nature not powerful enough.

Two examples from exceptions are relevant:

  • throwing \Exception is almost always wrong, and an argument could be made for preventing it being thrown by userland code. However this would make testing applications be very annoying, as you could not test against the generic exception being thrown.

  • catching \Exception and not doing anything with it is almost always wrong. Except in the few cases where it isn't.

Any error handling system can be abused. The position of this RFC is that we should make it easy for people to write good quality code, not to force a particular set of quality on them.

1 - https://github.com/ircmaxell/ErrorExceptions/blob/master/lib/ErrorExceptions/CoreExceptions.php

2 - http://slurm.schedmd.com/fair_tree.html#algorithm

numeric event handlers in Maple - http://www.maplesoft.com/support/help/Maple/view.aspx?path=NumericEventHandler

https://docs.oracle.com/cd/E19957-01/806-3568/ncg_handle.html

Counterpoint

Allow a custom error handler to be injected via a function. This would also be very favorable, as it gives total control over how errors are handled. An error handler could be set globally for the extension or on a per-connection basis. In the case of an error, this error handler would get called and the function would return false. There is already the possibility to create a phpiredis_reader from PHP code, and a function to set an error handler on a reader also exists. I could however not find a way of telling phpredis_command_bs() for example, to use this specific reader. Is there a way of doing this? It seems like these readers are never used.

@ikariiin
Copy link

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment