👍

@trowski This looks even better, I think. 👍

the only concern of mine here is, the Error is too simple, it probably be wildly used...

but I admit that this is much better than current implementation.

For catching all errors, catch(Throwable $e){} sucks. The name semantically is terrible:

catch(Throwable $stone){ // WAT
    // you know, consistency
    $this->getMedievalLogger->siege()->logThrew($stone);
}

It doesn't say anything about the error severity. It's fine for tools such as PHPUnit but catch(Throwable $e) is definitely not something you want to read in regular applications.

In my opinion there should be only one major type and it's Error:

• class Error
  • class TypeCheckError extends Error
  • class AssertionError extends Error
  • class Exception extends Error

Focus on this pls: At the end of the day, an unresolved/unhandled Exception is an Error that you want to log/debug. If an Exception doesn't get caught by any catch(Exception $e){} block, will then be caught by a super catch(Error $e){} block; which basically means that the Exception, as it failed to get resolved in the relevant stack, gets "promoted" to Error status.

if you don't like Error, Failure could be fine:

• class Failure implements Throwable
  • class TypeCheckFailure extends Failure
  • class AssertionFailure extends Failure
  • class Exception extends Failure

Also, i find TypeCheckFailure|Error more descriptive than just TypeFailure|Error, just because at some point we might want to have also TypeCastError (like array to string) or things.

also don't forget about (current name) AssertionException. as I just told you in room11, failed preconditions are considered programming errors, so:

• class Failure implements Throwable
  • class TypeCheckFailure extends Failure
  • class AssertionFailure extends Failure
  • class Exception extends Failure

try{
    try{
          assert(isset($lol))
          $foo = $this->getFoo($lol);
    }catch(Exception $e){
          $foo = $this->createFoo($lol);
    }
    $foo->run();
}catch(Failure $e){
    $logger->log($e);
}

@WesNetmo:

class Exception extends Failure? I don't think that exception is neccessary failure, it's just... an exception.
We need to introduce an interface Throwable (or some interface with similar name e.g. an IException), to separate different exception hierarchies from themselves, because they are really semantically different. Engine exceptions completely differs from these userland, and from, for example, -Failures, is it so? I think yes.

I will start a discussion on internals or room 11 if there's time today.

@Kubo2 i've already answered your question here #1284 (comment)

@WesNetmo Yeah, but from there I think the best name for the interface is IException (even if it still isn't really big win).

@Kubo2 no pls. this is not c#

The name Error was chosen to correspond with PHP's other errors. Non-fatal errors detected by PHP will continue to trigger warnings and notices, while fatal errors are thrown as Error exceptions.

Conceptually both of these conditions are error conditions detected by PHP. The only difference is that for some errors the execution of the script can continue from where the error occurred; for others it is not possible for execution to continue from the place where the error occurred, and so instead an exception must be thrown.

While this name may also cause some confusion for users, other name choices such as Failure do not seem appropriate. It is likely that users would use the term 'Uncaught Error' when searching, minimizing overlap with with non-fatal error messages.

What is the status of this? Looks like discussion has settled down, and PHP 7 Alpha 1 should be tagged in 9 days from now, would be nice to get this change in (or Alpha 2 at least).

@Majkl578 Voting for this RFC ends on June 24th. I guess we will see this in the next release after that date.

I know, my question is 13 days old, at that time, there was not even a voting proposal. A bit unfortunate that the vote ends one day after alpha 2 deadline [1] - maybe it could be postponed by one day to get this change in? cc @weltling @KalleZ

[1] http://news.php.net/php.internals/86605

@weltling No problem, I'll do this later today.

@weltling All conflicts resolved and tests updated to use revised exception names, plus updated the previous parameter to accept any Throwable instead of just an Exception. I also migrated AssertionException to AssertionError.

@trowski, please don't change this pull request, i've already started to merge/test what was in prior to the Andrea's comment. You can create another PR after this one is merged.

Thanks.

I was changing it at the request of @bwoebi and @nikic. They felt the macros should be renamed to clearly indicate they were not for the general registration of interfaces.

@nikic, @bwoebi was you intended to merge this PR? Then i'll just retain inactivity :)

Thanks.

@trowski, merged, thanks for your work!

🚀

🆗 👯

@trowski Can you please explain when will ParseError be thrown (maybe by eval() ?) and when will TypeError be thrown ?

I couldn't find such information on this thread or on the RFC.

Thanks :) .

@SoboLAN I wrote a short blog post about the new exception hierarchy in PHP 7 that answers your questions: Throwable Exceptions and Errors in PHP 7

Is there any chance to extend the interface with a addSuppressed() method? Semantic should be the same as in Java's Throwable::addSuppressed()

Just consider following code fragment:

    } catch (SomeOtherException $e1) {
        try {
            $this->pdo->rollback();
            throw new \DomainException("Failed", 0, $e1);

        } catch (\PDOException $e2) {
            // $e2 is lost!
            throw new \DomainException("Failed even more", 0, $e1);
        }
    }

The last catch block has now a $e1 and $e2 which need to be propagated by the DomainException. But $e2 gets lost.

@malkusch This could easily be implemented in user code:

interface ThrowableWithSuppressed extends Throwable
{
    public function addSuppressed(Throwable $exception);
    public function getSuppressed(): array;
}

class MyException extends Exception implements ThrowableWithSuppressed
{
    private $suppressed = [];

    public function addSuppressed(Throwable $exception)
    {
        $this->suppressed[] = $exception;
    }

    public function getSuppressed(): array
    {
        return $this->suppressed;
    }

    public function __toString(): string
    {
        $string = parent::__toString();

        if (!empty($this->suppressed)) {
            $string .= "\n";
            foreach ($this->suppressed as $exception) {
                $string .= "\nSuppressed " . $exception;
            }
        }

        return $string;
    }
}

try {
    throw new Exception('1');
} catch (Exception $e1) {
    try {
        throw new Exception('2');
    } catch (Exception $e2) {
        $e = new MyException('3', 0, $e1);
        $e->addSuppressed($e2);
        throw $e;
    }
}

Note that if an exception is created while a thrown exception already exists, the previously thrown exception is automatically set to the previous exception. For example:

try {
    throw new Exception('1');
} finally {
    throw new Exception('2');
}

Fatal error: Uncaught Exception: 1 in %s:%d
Stack trace:
#0 {main}

Next Exception: 2 in %s:%d
Stack trace:
#0 {main}
  thrown in %s on line %d

This could easily be implemented in user code

Sure, but this doesn't help if I have to handle cases which are out of my domain.

Note that if an exception is created while a thrown exception already exists, the previously thrown exception is automatically set to the previous exception.

Oh that's sweet. I didn't expect that. Thank you, that's very good to know. But don't you think that's odd? There's no causality between Exception("1") and Exception("2"). But maybe that's OK as in PHP it's called previous exception without any further specification of that semantic. So this might be acceptable.

Sure, but this doesn't help if I have to handle cases which are out of my domain.

Libraries that have patterns like you've outlined could define an exception class with a list of suppressed exceptions, otherwise it isn't necessary.

However, I see your point, as it would be a common pattern and therefore might be useful to be included in the base exception classes.

But don't you think that's odd? There's no causality between Exception("1") and Exception("2").

Admittedly it is a little magical that the first exception is set to the previous exception for the second, but isn't much different than magically adding the exception to a list of suppressed exceptions.

I see you started a discussion on internals, so this can continue there.