safe icon indicating copy to clipboard operation
safe copied to clipboard

fgets() is missing

Open BenMorel opened this issue 5 years ago • 5 comments

Hi, it looks like common file functions such as fread() and fwrite() are present, but not fgets(). Could you please add it? Thanks!

BenMorel avatar Jan 22 '20 10:01 BenMorel

Hello, I am not sure this will be possible. Looking at the the doc: https://www.php.net/manual/fr/function.fgets.php, The function can also return FALSE once there is nothing left to read.

All our generated functions can do is only checking for the return value to know if an exception can be thrown. We would need to find another to know if an error happens and write a special case for this function.

I will try to look into it once I have some time

Kharhamel avatar Jan 24 '20 13:01 Kharhamel

Thanks for your reply.

I've been actually surprised to see that your implementation only checks the return value, and does not systematically throw when a PHP error has been caught.

In the case of fgets(), a warning is thrown if an error occurs, that's how you can differentiate from a false value meaning nothing left to read, or a false value meaning an exception.

I've done something like this here:

https://github.com/brick/std/blob/0.3.0/src/Io/FileStream.php#L96

In the case of fgets(), I consider false as a non-exceptional return value. If an error occurs, the warning will be caught and converted to an exception. So the return value is not taken into account at all for exceptions.

In the case of fwrite() however, I consider false as an exceptional return value. As before, if an error occurs, the warning will be caught and converted to an exception. And if no warning is caught but false is returned, an exception is also thrown.

BenMorel avatar Jan 24 '20 13:01 BenMorel

I've been actually surprised to see that your implementation only checks the return value, and does not systematically throw when a PHP error has been caught.

Yes it is definitely much more limiting, but also easier to implement automatically and it doesn't risks messing with people's error handlers. Generally, the goal of this library isn't to rewrite the whole standard library, only to simplify error gestion a bit.

This subject isn't new, see #77 and #120

Kharhamel avatar Jan 24 '20 14:01 Kharhamel

We have a special case file where we can manually write safe versions of functions. However, i don't think we can use the code you linked, because it is kind of a design choice to not use errors handlers in safe.

Kharhamel avatar Jan 24 '20 14:01 Kharhamel

Hmm this is a matter of opinion I guess. I really expected this library to catch every single PHP error and re-throw it. I would personally expect Safe to bypass any custom error handler, for the duration of the function.

The fact is, in this precise case, it's really a pain to have fgets() fail because of an I/O error, and not get a catchable exception.

BenMorel avatar Jan 24 '20 14:01 BenMorel