Re: [RFC] Stream Error Handling Improvements

Hi Bob, On Wed, Nov 19, 2025 at 5:37 PM Bob Weinand <bobwei9@hotmail.com> 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