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 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.
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.
This also solves your gen-stub issues by making them unncessary.
Should StreamException be attached the non-terminal errors which are
caused by the terminal errors? I.e. StreamException clearly is
STREAM_ERROR_CODE_PERMISSION_DENIED for example, but it happens
during a
copy, so, should the information that a STREAM_ERROR_CODE_COPY_FAILED
was caused by that, also be present on the StreamException?
I think I get what you mean from the user point of view but I'm not
sure how could this be reasonably linked.
I could maybe check during stream error if there is EG(exception) it's
a StreamException and if so, I could the update it with this error
(e.g. some property that would hold those error can could be
retrieved). I guess it could be potentially extended for terminal
errors as well so it could have method like getAllErrors which would
return all errors because sometimes there can be more terminal errors
and some of them might get lost (last one will be added). However this
would work just for exception so maybe more generic solution is needed
here (see below).
Replying below to this.
Are stream errors which happen as part of a fully wrapped
functionality
marked as wrapper error as well? Like if the server sent an
invalid TLS
response to a file_get_contents("https://...";) operation, making the
whole operation fail? You obviously don't have access to the
intermediary internal stream resource there, where the stream
error is
attached.
I actually thought about this yesterday that it's not ideal that some
errors might get lost in this way so I think it should go to wrapper
errors as well.
If we properly wrap the nested operations into a single error, then it
would be automatically part of the wrapper error, with no need to
explicitly add it to stream error.
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.
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 also like the suggestion by Jordi to pass the StreamError right away
into the user handler.
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.
The only disadvantage of top-level wrapping is probably a bit more
book-keeping as in "storing that there is an outer operation before
invoking an inner operation"? Haven't thought much about the actual
implementation of this, but should be doable.
Thanks,
Bob
