Hi Bob,

On Wed, Nov 19, 2025 at 5:37 PM Bob Weinand <[email protected]> wrote:

> Hey Jakub,
>
> thanks for the detailed reply.
> On 19.11.2025 12:52:35, Jakub Zelenka wrote:
>
> The code one should be probably named StreamErrorCode (to not interact
> with what Jordi proposed) and think it will need to be backed as there
> should be a straightforward way how to compare it with exception code. I
> might need to extend the gen stub as I'm not sure it allows macro import
> for enum value like for constants or I will need to come up with some
> mapping. I just don't wan to duplicate the numbers. I will figure something
> out.
>
> How important is it to have exception code?
>
I think the exception code could be quite useful for checking for specific
errors. It's much better than trying to match it by error message which I
saw in past used in normal error handlers. So I can see definitely use case
for that. It might be also possible to group those erors if enums are used
so users could just match specific group of errors.

> I found that for our purposes we generally skip the exception code (i.e.
> keep it at zero).
> As a simple example, ErrorException does take two integers - one for
> severity, and one for code, instead of using the severity as code.
>
Severity is almost always warning which I don't think is a good
representation for code. I saw error codes used in various application and
I think they are useful. I don't think it hurts to have them. The
implementation of that enum should not be a big problem. I just need to
figure out how to make it nice.

> I would propose something similar here: call it StreamError and provide a
> public $error property for the actual enum, rather than squeezing it into
> $code.
>
I'm not sure I understand this. The code is a generic identifier for the
error class that can be matched. StreamError would be the actual
representation containing the message and other info - this cannot be
matched in a generic way.

> Is this what the logged errors section is about? Is that section talking
>> about "php_stream_display_wrapper_errors", which can concat some errors?
>> I see that this logic is mostly retained.
>>
>
> Yeah the logged errors is an existing mechanism for grouping errors to a
> single one (concatenating message). The display is a bit messy because it
> separated them by new line or <br/> (if html errors enabled) but I just
> kept it to limit BC impact.
>
> I don't think changing the display / grouping here would be problematic
> with respect to BC. But I get that this initial draft tries to limit what
> it changes.
>
Exactly I would like to limit the external changes.

>
> Do I understand the implementation correctly, that user error handlers
>> can be called multiple times for a same operation? E.g. when copying a
>> file, you'll get first a PERMISSION_DENIED then a COPY_FAILED? Should
>> these not be somehow merged? Especially with global user_error handlers,
>> these cannot be aware of the fact that the permission denied is part of
>> anything else - at least not until possibly a second user_error
>> invocation happens.
>>
>
> This is somehow related to the exception error grouping as it requires
> some sort of linking between the errors.
>
> I guess to make this work we would need some top level wrapping similar to
> what logged errors do but instead of concatenating, it would group the
> errors in a better way which could then be used in exception or handler.
> This would, however, increase the complexity significantly as it is
> possible to nest the stream calls (e.g. stream calls in notification
> callback or in the new error handler) so there would probably need to be
> some stack. Also I'm not sure about the API. Should we pass always array of
> errors to the callback (which would mostly have just one element anyway or
> somehow chain the StreamError object (something like previousError()) and
> what message should be used in exception (e.g. last error and then helper
> function to get all errors)?
>
> I'm willing to look into this but would like to know if this is something
> important for user space and what is the usual flow how could this be used
> in frameworks and so on. In other words I would like to get more input to
> make sure that this is really needed as it requires quite a bit of effort
> to do.
>
> Doing this work will definitely tidy up the whole error story. It's not
> fundamentally crucial, but it definitely will make for a nicer API.
>
> And yes, chaining StreamError sounds and nesting exceptions to $previous
> sounds like the most straightforward way.
>
> Also a possibility for exceptions: this might be a bit of work, but you
> could insert a fake frame for exceptions to indicate when an error appears
> as a sub-operation. Like:
>
> StreamException: Permission denied
> #0 [internal function]: fopen("target.php", "w+")
> #1 [internal function]: copy("source.php", "target.php")
> #2 ...
>
> I will check this out.

> I also like the suggestion by Jordi to pass the StreamError right away
> into the user handler.
>
>
> Yeah agreed.


> Also, is the internal resource used for e.g. file_get_contents() leaked
>> into user error handlers? I'm not sure whether that's desirable. If
>> that's intended, that's fine too, I think.
>>
>>
> Currently yes. I need to actually double check the consequences of that as
> I didn't really think about internal resources.
>
> If we decide to go with completely wrapping and calling the user error
> handler only once per top-level operation, this point would be moot anyway.
>
>
>
> Does stream_get_errors() ever reset? Looking at the code it doesn't seem
>> so. It will just forever grow. E.g. fwrite() will report an error every
>> single time when the buffer is full and a write is attempted. If errors
>> aren't freed, that's just a memory leak (as long as the socket lives).
>> This is a serious flaw.
>>
>> Lastly, stream_get_errors() is overkill in a lot of cases (especially
>> when using fread()/fwrite() on simple sockets). I just care about the
>> last error code (there's just going to be one anyway). I'd love having a
>> stream_get_last_error() returning just the code.
>>
>>
> I guess this makes more sense but it kind of also implies the top level
> wrapping as it should contain all errors in the call. Or alternatively it
> could be just a ring with limited number of errors be behave in the same
> way like openssl_error_string. I guess it depends if we go with that top
> level wrapping.
>
> I feel like the top-level wrapping would simplify things a lot (from point
> of addressing shortcomings), and if this just works like
> json_get_last_error(), where you only ever see the last error, there's no
> risks of being unbounded nor having arbitrary size limits.
>
Yeah I think it would be more logical and improve things so I will check it
out and see when I have my next stream errors slot (should be later next
month).

Cheers,

Jakub

Reply via email to