# Error instead of returning false - Thread: https://externals.io/message/105626 - Original archive: https://news-web.php.net/php.internals/105626 - Started by Gert on 2019-05-07T14:22:07+00:00 - Messages: 9 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #105626 — Gert — 2019-05-07T14:22:07+00:00 URL: https://externals.io/message/105626#105626 Hello internals, I wanted to propose an RFC, but i'd like to check first if this idea has been considered before already. My idea, extremely summarized, would be to take the functions that return false/null when they 'error', and instead make them actually throw an error. Currently, functions like getcwd or json_decode will return false/null when an error occurs, and they populate a global error state, which can be retrieved by error_get_last, or similar functions. json_decode already got the `JSON_THROW_ON_ERROR` option in PHP 7.3, and this RFC would basically make that behaviour default. This RFC would target PHP 8.0, but i'd like to hear some opinions on this. Greetings, Gert de Pagter (BackEndTea) --- ## #105627 — Chase Peeler — 2019-05-07T14:52:24+00:00 URL: https://externals.io/message/105626#105627 > Hello internals, > > I wanted to propose an RFC, but i'd like to check first if this idea > has been considered before already. > > My idea, extremely summarized, would be to take the functions that > return false/null when they 'error', and instead make them actually > throw an error. > > Currently, functions like getcwd or json_decode will return false/null > when an error occurs, and they populate a global error state, which > can be retrieved by error_get_last, or similar functions. json_decode > already got the `JSON_THROW_ON_ERROR` option in PHP 7.3, and this RFC > would basically make that behaviour default. This RFC would target PHP > 8.0, but i'd like to hear some opinions on this. > > This would be a pretty big BC break for those functions. That's not necessarily a bad thing, but, I think the burden for justification is larger than just adding a new feature. These are the questions I'd ask 1.) What is detrimental about the current behavior? 2.) What is beneficial about the new behavior? 3.) Are the items for #1 and/or #2 big enough to justify breaking existing code? 4.) Is there a way to achieve the desired behavior without a BC break. My initial thoughts are that impacts of the BC break would be pretty big. EVERYONE using json_decode without `JSON_THROW_ON_ERROR` will have to modify their code. It's not a quick find/replace change either. Instead of checking the return, and then checking the error state (or, maybe they are OK with just continuing if there is an error), now has to wrap the call into a try/catch block. I personally don't see anything that detrimental about the current behavior or that beneficial about the new behavior. It's not like try/catch is that much simpler from an implementation standpoint that checking a global error state based on the return value: $foo = json_decode($bar); if(false === $foo){ //get error and do whatever to handle it } vs try { $foo = json_decode($bar); } catch (\Throwable $t){ //do whatver to handle it } It also appears we can easily provide the functionality you desire with additional options. You mentioned that json_decode has already been given a new option, JSON_THROW_ON_ERROR. What is the advantage to making this the default behavior vs letting people specify the flag when they want? If there a reason adding such an option as a new parameter wouldn't work for other methods, like getcwd? > Greetings, > Gert de Pagter (BackEndTea) > > -- > > > > -- Chase Peeler chasepeeler@gmail.com --- ## #105628 — Nikita Popov — 2019-05-07T15:09:38+00:00 URL: https://externals.io/message/105626#105628 > Hello internals, > > I wanted to propose an RFC, but i'd like to check first if this idea > has been considered before already. > > My idea, extremely summarized, would be to take the functions that > return false/null when they 'error', and instead make them actually > throw an error. > > Currently, functions like getcwd or json_decode will return false/null > when an error occurs, and they populate a global error state, which > can be retrieved by error_get_last, or similar functions. json_decode > already got the `JSON_THROW_ON_ERROR` option in PHP 7.3, and this RFC > would basically make that behaviour default. This RFC would target PHP > 8.0, but i'd like to hear some opinions on this. > > Greetings, > Gert de Pagter (BackEndTea) There's two cases to distinguish here: 1. Error conditions that indicate a programmer error (what we would throw an Error exception for nowadays). 2. Error conditions that indicate a data/environment/etc error (what we would throw an Exception exception for nowadays). One large category of warnings of the first kind has already been converted to TypeErrors in https://wiki.php.net/rfc/consistent_type_errors. And I'd be in favor of changing more of these in PHP 8. This is (relatively) non-intrusive from a BC perspective, because these conditions are not something you should ever explicitly check for: If your code hits them, it is simply broken. However, what you seem to have in mind here are error conditions of the second kind. I don't think we can change these on any timeline. Current code (that is careful about handling errors) will be checking false/null return values for them, and throwing an exception instead would be a major and very hard to automatically fix BC break. The proper way (imho) to handle an eventual migration to exceptions is to introduce new APIs that supersede the existing ones. E.g. if we ever get around to introducing an OO API for the stream layer, than that would be the time to also introduce use of exceptions. A possible alternative would be to control this behavior in some way similar to declare(strict_types). Regards, Nikita --- ## #105629 — Mark Randall — 2019-05-07T15:14:03+00:00 URL: https://externals.io/message/105626#105629 > My idea, extremely summarized, would be to take the functions that > return false/null when they 'error', and instead make them actually > throw an error. It comes up on various discussion forums every other month. There's a few downsides to it, but mainly that any existing error handling would need to be re-written as tests for the false value would never be reached after the exception was thrown. It's a bit less of a hassle for more modern PHP design practices, as any warnings are globally promoted to exceptions using set_error_handler in userspace, but this too has its problems (for example in third party libraries). IIRC, most functions are at least checking their input argument types now and throwing InvalidArgumentExceptions, and the odd few like json_decode have their exception behaviour, provided its flagged. The new security focused stuff always throws (such as if there's not enough entropy available). I don't think it's in any way practical to add throw-or-not flags to every other internal function that could error out, and so there are 4 options that I can see: 1. Make no change. 2. Everything throws. People have to re-write. The compiler would probably need to further inspect the AST at compile time, and throw a depreciation warning if the result of an error-able internal function was tested for false. 3. Class (or namespace) level declares to throw on internal function errors. 4. Leave the existing functions as-is, finally claim the \php namespace, alias all of the internal functions into it, and make their default, unchangeable behaviour be to throw a RuntimeException on error. -- Mark Randall --- ## #105630 — Claude Pache — 2019-05-07T15:15:41+00:00 URL: https://externals.io/message/105626#105630 > Le 7 mai 2019 à 16:22, Gert a écrit : > > Hello internals, > > I wanted to propose an RFC, but i'd like to check first if this idea > has been considered before already. > > My idea, extremely summarized, would be to take the functions that > return false/null when they 'error', and instead make them actually > throw an error. > > Currently, functions like getcwd or json_decode will return false/null > when an error occurs, and they populate a global error state, which > can be retrieved by error_get_last, or similar functions. json_decode > already got the `JSON_THROW_ON_ERROR` option in PHP 7.3, and this RFC > would basically make that behaviour default. This RFC would target PHP > 8.0, but i'd like to hear some opinions on this. > > Greetings, > Gert de Pagter (BackEndTea) > > -- > > > The big issue, with your proposal, is, of course, migration of legacy code: If you do think that ideally an exception should be thrown here, it is better to devise a new function—and, while you’re at it, a function that returns an object of some definite class instead of a resource. Another issue is that a “failure” is not necessarily an “error”. For example, when I use `strpos()`, I expect that it returns sometimes `false`; and either using a try/catch construct, or using a separate check beforehand would be cumbersome. —Claude --- ## #105631 — Stanislav Malyshev — 2019-05-07T18:46:12+00:00 URL: https://externals.io/message/105626#105631 Hi! > My idea, extremely summarized, would be to take the functions that > return false/null when they 'error', and instead make them actually > throw an error. First of all, they definitely should not throw Error, because Errors are for very specific types of things that mean the engine does not know what to do - e.g. if you asked for an integer and you gave parameter that can not be converted to integer, or you tried to use a function which does not exist, and so on. Normal functionality should almost always use Exception unless you have a really great reason not to. Second, I don't see any point in such a huge BC break for people that are fine with current return values. Exceptions is not something you can ignore (even if you can easily ignore method returning false - sometimes you don't want to, but sometimes you do), it's something that breaks your application. In some APIs for some corner cases it might be OK, but in general it's probably not something we'd ever want to do on a global scale. Third, I think throwing exceptions for routine situations that are expected to happen (like file not being there, or data not being valid) is wrong and this is what makes Java exceptions, for example, annoying. They confuse "everything broken and world is on fire, head for the exits" with "there's some dirt on the floor, bring the mop and clean it up". I.e. situations which are expected to be handled and situations where things are just gone wrong and it's best for this code do give up immediately. Exceptions should be used for situations which are, well, exceptional - ones that the code surrounding the call is unlikely to be able to fix - e.g. trying to open file while giving the argument that can not be used as a filename. Of course, the question what is "normal" for a given API is very specific to the API - e.g. for a search API not finding any result would be normal, but for an authorization API not finding certain record in the database may be exceptional - but I think there should be a distinction between the two. Which should be used in each specific case depends a lot on API design and specifics of the case, but there should be an option for both behaviors. -- Stas Malyshev smalyshev@gmail.com --- ## #105634 — Eugene Leonovich — 2019-05-07T19:57:18+00:00 URL: https://externals.io/message/105626#105634 > > > […] > > It comes up on various discussion forums every other month. There's a > few downsides to it, but mainly that any existing error handling would > need to be re-written as tests for the false value would never be > reached after the exception was thrown. > > It's a bit less of a hassle for more modern PHP design practices, as any > warnings are globally promoted to exceptions using set_error_handler in > userspace, but this too has its problems (for example in third party > libraries). > > IIRC, most functions are at least checking their input argument types > now and throwing InvalidArgumentExceptions, and the odd few like > json_decode have their exception behaviour, provided its flagged. The > new security focused stuff always throws (such as if there's not enough > entropy available). > > I don't think it's in any way practical to add throw-or-not flags to > every other internal function that could error out, and so there are 4 > options that I can see: > > 1. Make no change. > > 2. Everything throws. People have to re-write. The compiler would > probably need to further inspect the AST at compile time, and throw a > depreciation warning if the result of an error-able internal function > was tested for false. > > 3. Class (or namespace) level declares to throw on internal function > errors. > > 4. Leave the existing functions as-is, finally claim the \php namespace, > alias all of the internal functions into it, and make their default, > unchangeable behaviour be to throw a RuntimeException on error. > > -- > Mark Randall > > -- > > > > For the record, there is already a library for option 3: https://github.com/thecodingmachine/safe Another option would be to introduce a new declare directive, something like > > If you do think that ideally an exception should be thrown here, it is better to devise a new function—and, while you’re at it, a function that returns an object of some definite class instead of a resource. Isn’t this exactly what SplFileObject / SplFileInfo::openFile is for? --- ## #105647 — Dan Ackroyd — 2019-05-08T19:21:36+00:00 URL: https://externals.io/message/105626#105647 > > Isn’t this exactly what SplFileObject / SplFileInfo::openFile is for? SplFileObject does not expose the underlying filehandle, so it can't be used with any of the other PHP functions that expect a file handle. It's not safe to expose the filehandle, as none of the SplFileObject code checks for the file handle still being valid. cheers Dan Ack