[RFC] Throwable Hierarchy Policy for Extensions

Hi

The exception message MUST NOT be the only property that allows to
differentiate different types of error that the user may be interested in.

What does this mean exactly? Can you give an example?

This is intended to say “if a user calls ->getMessage() and consumes it
in an if() statement, then you are doing it wrong” and “the error
message is intended for human consumption and changes to the message are
not considered breaking changes”.

Ideally different exception classes should be used, but using the
$code property to differentiate between different types of error is
also acceptable when there is a wide range of errors the user might be
interested in. For PDO this might be:

 PdoException extends Exception
 PdoError extends Error
 UnsuccessfulQueryException extends PdoException
 QuerySyntaxError extends PdoError

And then in UnsuccessfulQueryException, use a different code for
"Duplicate entry", "Query timed out", and "Deadlock" to avoid adding a
separate class for each failure case. It might make sense to add classes
for the failure cases that are most likely to require special handling.
e.g. a DeadlockException to retry the transaction.

But as a user I should not need to do:

 again:
 try {
     $query->execute();
 } catch (UnsuccessfulQueryException $e) {
     if (str_contains($e->getMessage(), 'Timeout')) {
         goto again;
     }
     throw $e
 }

to determine whether it was a timeout, a deadlock or a duplicate entry.

Do you have a wording suggestion to make it clearer what is meant by
that sentence?

Best regards
Tim Düsterhus

The exception message MUST NOT be the only property that allows to
differentiate different types of error that the user may be interested in.

What does this mean exactly? Can you give an example?

Many people don’t know about the $previous property, which allows you to chain exceptions, for example. I’ve worked in more than one codebase with custom exceptions that are missing that. It makes for throwing from a catch more nebulous as you lose how you got there in the first place.

— Rob

• Any two exceptions with different causes MUST be identifiable either
  by a unique exception class name, a stable $code, or a
  class-specific additional property suitable for programmatic
  consumption (e.g. an enum).

I would probably not even allow the stable $code in here, as I have
seen from experience people don't really check for them.

The only time I've seen anyone use $code is in TYPO3. Their coding standards say that any time you throw an exception, you use the current timestamp (determined manually) as a code. That way there is a globally unique code regardless of exception type that can be grepped to find the exact line it came from.

I am not saying this is a good strategy, just that it's the only time I've seen $code used in the wild...

--Larry Garfield

Hi

• Exceptions MUST NOT be final.

Could the RFC explain why not?

I'm not sure if this is useful to add to the RFC itself as a “only extra
explanation” and since the discussion is an equally official resource:

The reason is to allow flexible extensions of the exception hierarchy,
e.g. when adding a more specific type of exception to provide further
context to some parent exception that just identifies some “concept”.

A HTTP request can fail (HttpRequestFailedExceptions) for different
reasons, e.g. due to a connection failure (ConnectionFailedException) or
due to a server error (ServerErrorException). Now we might also want to
clarify why the connection failed. It could be a timeout
(ConnectionTimeoutException) or DNS resolution error
(DnsResolutionFailedException). This already requires several of the
exceptions not to be final to allow extending them. Making the leaf
exceptions final would not bring any value and just cause additional
churn when realizing that having more child exceptions would be helpful
to the user. Also some extension might intentionally want to allow
subclassing for some classes. When subclassing a class it also makes
sense being able to subclass the corresponding exceptions. I have made
it legal to throw “unowned” exceptions when subclassing something in:

https://github.com/php/policies/pull/17/commits/1a125dabea93b4c919677065651c4fdea1b035e3

• The name of the extension SHOULD NOT be used as a prefix or suffix of
  the unqualified class name of additional exceptions.

Could you add an example of how to do it instead (or a "not this" "but
that" example)?

I've expanded on this paragraph and added an example in:

https://github.com/php/policies/pull/17/commits/c0e8891fc9a7a459a26a990b33108bfbd5ae3311

Basically the intention is to avoid class names “oddly specific” names
that just concatenate some random words. Ideally the class name would be
a succinct English phrase that matches what you would communicate them
to a co-worker. I would say “The HTTP request failed”, but not “The HTTP
request that we perform using the network request library called curl
failed”. Or I would say “The timezone is invalid” rather than “The
timezone, which relates to the concept called ‘date’ (and not any other
use of the term timezone) is invalid”.

How exactly that works in practice greatly depends on the extension,
that's why it's just a SHOULD (NOT). I trust that folks make good
choices when they have a reminder to make a good choice.

• Any two exceptions with different causes MUST be identifiable either
  by a unique exception class name, a stable $code, or a
  class-specific additional property suitable for programmatic
  consumption (e.g. an enum).

I would probably not even allow the stable $code in here, as I have
seen from experience people don't really check for them.

My goal here is to avoid making exception messages part of the backwards
compatibility promise. Whether or not the $code is useful in practice
will be something that folks can figure out when writing an RFC. It
probably greatly depends on the type of extension what makes sense.
Perhaps it would also make sense to officially widen the code from int
to int|string|UnitEnum to avoid the PDO gotcha. Since __construct()
does not participate in LSP checks and since getCode() is already
final, this seems safe to me.

Best regards
Tim Düsterhus