Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127256
Message-ID: <a11c1087-1d1d-4cf3-b047-4c76fc676632@bastelstu.be>
Date: Wed, 30 Apr 2025 20:53:30 +0200
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PHP-DEV] [RFC] Throwable Hierarchy Policy for Extensions
To: Derick Rethans <derick@php.net>
Cc: PHP internals <internals@lists.php.net>,
 "Gina P. Banyard" <internals@gpb.moe>
References: <ffadb6e6-5d16-4b45-a020-4b91565e2ee6@bastelstu.be>
 <4233b00a-7593-e70d-f235-da650cd7bac6@php.net>
Content-Language: en-US
In-Reply-To: <4233b00a-7593-e70d-f235-da650cd7bac6@php.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=C3=BCsterhus?=)

Hi

On 4/30/25 13:18, Derick Rethans wrote:
> - 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