Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:130480
Precedence: list
MIME-Version: 1.0
References: <CAEKnhAEO8w0GJXTVyH_nwK8nGQGdJcNNkGOPMnYDVrcsNGrm4A@mail.gmail.com>
 <CAEKnhAGCxHOTOmtU7c8rqm3MY8a4vvoJ3NH7BdR0SNp+7sFWHg@mail.gmail.com>
 <CAOWwgpk5wpyWo-qeVO4mGwKygTMFMeXDt3_1Cf4wLOK4ZgZaSw@mail.gmail.com>
 <CAEKnhAHFKYbXMm7LU+-yexLgwRmw=fPx8b5vUVNOQUg5AjRbaw@mail.gmail.com> <f49931e8334d6c745d5daa5afd4ebcd8@bastelstu.be>
In-Reply-To: <f49931e8334d6c745d5daa5afd4ebcd8@bastelstu.be>
Date: Sun, 29 Mar 2026 20:25:49 +0200
Message-ID: <CAEKnhAGPUowhK8z1MKGqkU3otk0oi0pqMu4=UO_V3JGQK9LscQ@mail.gmail.com>
Subject: Re: [PHP-DEV] Re: [RFC] Stream Error Handling Improvements
To: =?UTF-8?Q?Tim_D=C3=BCsterhus?= <tim@bastelstu.be>
Cc: Nicolas Grekas <nicolas.grekas+php@gmail.com>, 
	PHP internals list <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000b8f5d7064e2ddd9c"
From: bukka@php.net (Jakub Zelenka)

--000000000000b8f5d7064e2ddd9c
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Thu, Mar 26, 2026 at 5:57=E2=80=AFPM Tim D=C3=BCsterhus <tim@bastelstu.b=
e> wrote:

> Hi
>
> thank you for your RFC. I'm regretfully late to the party, but my
> primary comment about the RFC relates to the latest changes.
> Specifically:
>
> Am 2026-02-27 23:21, schrieb Jakub Zelenka:
> > I changed it to the StreamError class constants and also move the
> > is*Error
> > functions there.
>
> The RFC specifically defines =E2=80=9Cranges=E2=80=9D for the numeric val=
ue of the error
> constants, some of which are quite small with only 10 values available.
> I believe this is dangerous, because it is possible to run out of values
> within a specific range and users might make assumptions based on the
> ranges when they really should rely on the `is*()` methods to check what
> type of error there is. This is not a problem that would exist if the
> error code was an unbacked enum.
>
> Contrary to Nicolas, I also don't think it is a problem for the error
> code enum to be extended in future PHP versions. In other programming
> languages, Rust specifically, it is also common for errors to be defined
> as an enum that is explicitly defined to not be exhaustive. Here's an
> example in the standard library:
> https://doc.rust-lang.org/std/io/enum.ErrorKind.html. Note how the
> documentation says:
>
>
I agree with this and as you pointed out privately we already have
precedent  in URI extension - enum UrlValidationErrorType:
https://github.com/php/php-src/blob/5e45c17d817df003cd24109f0ae222c5d82fecd=
1/ext/uri/php_uri.stub.php#L112-L143
.

The constant were also not user friendly as they were just numbers so we
would need some look up function to produce name. Especially painful for
logging which is probably the main use case here.

I changed it back to enum and this time I made it non backed which was
possible to do in a nicer way due to recent changes that Arnaud did for
declaration headers containing C enums. It makes the whole implementation
much nicer actually. I just extended the gen_stub.php to optionally produce
look up table for enum name which should speed up and simplify the look
ups.

As for the `StreamError` class being a linked list: I agree with the
> folks that mentioned that the returned errors should be an array
> instead. `count()` would naturally be available, `hasCode()` can be
> implemented with `array_any($errors, fn ($error) =3D> $error->code =3D=3D=
=3D
> CODE_DECODING_FAILED)` and the =E2=80=9Cprimary error=E2=80=9D can be obt=
ained with
> `array_first()` (which we added in PHP 8.5).
>
>
As you were the second person to ask for it and that linked list was not
really a PHP thing, I changed it as suggested. This changed API in more
places to accommodate for it but think for logging and other use cases, it
will be a bit nicer at the end.


> For the naming of `stream_get_last_error()`: Within PHP we have both
> `X_get_last_error()` and `X_last_error()`. The latter seems to be more
> common and also what I would prefer here, because the `stream_get_`
> prefix sounds to me like we would get something from a stream, but the
> returned value is not related to a specific stream, but rather a global.
>
>
Good point, I changed it but because it now returns array (no linked list),
it's called stream_last_errors(). I also added  stream_clear_errors for
explicit clearing which might be useful in some situations.

The RFC and the implementation is updated so please take a look!

Kind regards,

Jakub

--000000000000b8f5d7064e2ddd9c
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Hi,</div><br><div class=3D"gmail_quote gmail_quote_co=
ntainer"><div dir=3D"ltr" class=3D"gmail_attr">On Thu, Mar 26, 2026 at 5:57=
=E2=80=AFPM Tim D=C3=BCsterhus &lt;<a href=3D"mailto:tim@bastelstu.be">tim@=
bastelstu.be</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" styl=
e=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddin=
g-left:1ex">Hi<br>
<br>
thank you for your RFC. I&#39;m regretfully late to the party, but my <br>
primary comment about the RFC relates to the latest changes. <br>
Specifically:<br>
<br>
Am 2026-02-27 23:21, schrieb Jakub Zelenka:<br>
&gt; I changed it to the StreamError class constants and also move the <br>
&gt; is*Error<br>
&gt; functions there.<br>
<br>
The RFC specifically defines =E2=80=9Cranges=E2=80=9D for the numeric value=
 of the error <br>
constants, some of which are quite small with only 10 values available. <br=
>
I believe this is dangerous, because it is possible to run out of values <b=
r>
within a specific range and users might make assumptions based on the <br>
ranges when they really should rely on the `is*()` methods to check what <b=
r>
type of error there is. This is not a problem that would exist if the <br>
error code was an unbacked enum.<br>
<br>
Contrary to Nicolas, I also don&#39;t think it is a problem for the error <=
br>
code enum to be extended in future PHP versions. In other programming <br>
languages, Rust specifically, it is also common for errors to be defined <b=
r>
as an enum that is explicitly defined to not be exhaustive. Here&#39;s an <=
br>
example in the standard library: <br>
<a href=3D"https://doc.rust-lang.org/std/io/enum.ErrorKind.html" rel=3D"nor=
eferrer" target=3D"_blank">https://doc.rust-lang.org/std/io/enum.ErrorKind.=
html</a>. Note how the <br>
documentation says:<br>
<br></blockquote><div><br></div><div>I agree with this and as you pointed o=
ut privately we already have precedent=C2=A0 in URI extension - enum UrlVal=
idationErrorType:=C2=A0<a href=3D"https://github.com/php/php-src/blob/5e45c=
17d817df003cd24109f0ae222c5d82fecd1/ext/uri/php_uri.stub.php#L112-L143">htt=
ps://github.com/php/php-src/blob/5e45c17d817df003cd24109f0ae222c5d82fecd1/e=
xt/uri/php_uri.stub.php#L112-L143</a> .</div><div><br></div><div>The consta=
nt were also not user friendly as they were just numbers so we would need s=
ome look up function to produce name. Especially painful for logging which =
is probably the main use case here.</div><div><br></div><div>I changed it b=
ack to enum and this time I made it non backed which was possible to do in =
a nicer way due to recent changes that Arnaud did for declaration headers c=
ontaining C enums. It makes the whole implementation much nicer actually. I=
 just extended the gen_stub.php to optionally produce look up table for enu=
m name which should speed up and simplify the look ups.=C2=A0</div><div><br=
></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;=
border-left:1px solid rgb(204,204,204);padding-left:1ex">
As for the `StreamError` class being a linked list: I agree with the <br>
folks that mentioned that the returned errors should be an array <br>
instead. `count()` would naturally be available, `hasCode()` can be <br>
implemented with `array_any($errors, fn ($error) =3D&gt; $error-&gt;code =
=3D=3D=3D <br>
CODE_DECODING_FAILED)` and the =E2=80=9Cprimary error=E2=80=9D can be obtai=
ned with <br>
`array_first()` (which we added in PHP 8.5).<br>
<br></blockquote><div><br></div><div>As you were the second person to ask f=
or it and that linked list was not really a PHP thing, I changed it as sugg=
ested. This changed API in more places to accommodate for it but think for =
logging and other use cases, it will be a bit nicer at the end.</div><div>=
=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0=
.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
For the naming of `stream_get_last_error()`: Within PHP we have both <br>
`X_get_last_error()` and `X_last_error()`. The latter seems to be more <br>
common and also what I would prefer here, because the `stream_get_` <br>
prefix sounds to me like we would get something from a stream, but the <br>
returned value is not related to a specific stream, but rather a global.<br=
><br></blockquote><div><br></div><div>Good point, I changed it but because =
it now returns array (no linked list), it&#39;s called stream_last_errors()=
. I also added=C2=A0=C2=A0stream_clear_errors for explicit clearing which m=
ight be useful in some situations.</div><div><br></div><div>The RFC and the=
 implementation is updated so please take a look!</div><div><br></div><div>=
Kind regards,</div><div><br></div><div>Jakub</div></div></div>

--000000000000b8f5d7064e2ddd9c--