search

laminas / laminas-diactoros

PSR HTTP Message implementations
https://docs.laminas.dev/laminas-diactoros/
BSD 3-Clause "New" or "Revised" License
483 stars 63 forks source link

Cover invalid arguments to Stream constructor #178

Closed gsteel closed 7 months ago

gsteel commented 12 months ago
Q A
QA yes

Description

Xerkus commented 12 months ago

What exactly is this supposed to achieve? fopen() failing to open stream is covered by further checks.

This is in response to a question in our slack about fopen() with url that gives 404 where false is returned and warning is triggered, right? Expecting some outside error handler to convert warnings to exceptions is not normal and should not be done in tests. To deal with it properly error handler should be added around fopen() call.

gsteel commented 11 months ago

@Xerkus - Yes, I added these tests last night after talking to @ramsey on Slack. I couldn't find any tests that checked for that specific exception. Whether it's valid or not to consider user defined error handlers, I'll leave up to you. Feel free to merge or close.

Adding an error handler to /src/Stream to ensure that warnings from fopen are not suppressed and are correctly raised as Throwable would probably be a worthwhile addition.