Hi Gina, On Wed, Nov 19, 2025 at 12:14 PM Gina P. Banyard <[email protected]> wrote:
> On Tuesday, 18 November 2025 at 18:41, Jakub Zelenka <[email protected]> > wrote: > > Hello, > > I would like to introduce a new stream error handling RFC that is part of > my stream evolution work (PHP Foundation project funded by Sovereign Tech > Fund) : > > https://wiki.php.net/rfc/stream_errors > > > Hi Jakub, > > Thank you for working on this, this has been a long-standing pain point of > our stream layer. > However, I do have various comments about the design of this. > > Echoing multiple voices, please use enums rather than constants where > applicable, and use an object rather than an associative array for > individual stream errors. > As others have pointed out, the notion of (non-)"terminal" errors is not > clear, and I'm struggling to understand what is the point of it? > > I just answered this in replay to Bob. The non terminating errors are those that don't change return value of the function (it means not returning failure). Some of those errors are still useful and users should be notified but they are not critical for correct result. This was mainly introduced for exceptions as those cases should not throw but it still makes sense to somehow notify users (e.g. store the error). > How does this interact with the stream notification stuff? > (I guess surrounding PHP_STREAM_NOTIFY_SEVERITY_INFO and > PHP_STREAM_NOTIFY_SEVERITY_ERR) > > It doesn't interact with it. Although PHP_STREAM_NOTIFY_SEVERITY_ERR somehow overlaps with it but it's limited in use (currently used only in ftp and http wrapper). Obviously I couldn't just use notifications and I needed to keep them working so the only possibility was to introduce a new API that handles just errors. Going forward I'm not sure PHP_STREAM_NOTIFY_SEVERITY_ERR should be kept but that's more a future scope I guess. > Regarding "docref" what is the point of including this in the error? > This is mainly used with php_docref to generate links to INI settings by > referencing an XML ID that exists in doc-en, this does not seem applicable > here. > I guess you added this due to the two usages of "streams.crypto" as a > docref in main/streams/transports.c, but this ID doesn't exist, and I > fear this might never have worked properly. > As such, I would recommend just getting rid of this. > > Agreed I'm not going to expose it. I will keep it just for error mode to keep it the same but I guess it might be better to drop it completely in the future. > The last thing that I feel *very* strongly about is that there should only > be 3 modes: > > - Silent > - Warnings > - Exceptions > > > As such, the cases where we currently emit a notice should either be > promoted to warnings (which for some of them I don't even understand why > they are just notices) or removed (mainly looking at the one in > main/streams/userspace.c) > And the singular case which bailouts using E_ERROR should be converted to > always throw an exception. > Bailouts cause all sorts of issues due to the lack of unwinding, things > that exceptions solve. > This would also simplify both the internal, and userland API and get rid > of the confusing notion of "errors" meaning "diagnostics/warnings". > > My aim is really not to touch the current logic and the error classification except one case where reporting was done accidentally. I think that non terminating errors make some sense as they give some hint that something could be improved but it's not necessarily preventing the functioning. This is something that has been already there and I don't think it needs to be changed as part of this proposal. Cheers, Jakub
