Re: [RFC] [VOTE] PREG_THROW_ON_ERROR

On Thu, Sep 10, 2026 at 1:41 PM Tim Düsterhus <tim@bastelstu.be> wrote: > > Hi > > On 2026-09-07 07:16, Osama Aldemeery wrote: > > First...wrapping couples the exception you catch to the flag. > > Without it, preg_replace_callback() throws whatever the > > callback > > throws. > > With it, the same call always throws a PregException. > > So the flag silently changes which exception a caller has to handle, > > and the two have to move together: > > > > ``` > > try { > > preg_replace_callback( > > '/[0-9]{4}-[0-9]{4}-[0-9]{4}-[0-9]{4}/', > > function ($matches) { > > return mask_credit_card($matches[0]); // throws > > MaskException > > }, > > $contents, > > ); > > } catch (MaskException $e) { > > // becomes dead the moment the flag is added, and comes back the > > moment it's removed > > } > > ``` > > That is correct, but as I mentioned before, the new flag is an entirely > new feature that requires an explicit opt-in. Adopting a new feature > without reading the associated documentation to find out how it works > will generally result in sadness, and I believe this case is no > different. Also adding and removing the flag would not just affect > MaskException, but would of course also affect whether or not a > PregException is thrown and whether or not the code proceeds > after an > error was encountered. Any existing error handling would need to adapted > as well. So the changes required to adopt the flag are much more > far-reaching than whether or not a catch block for a custom exception > would need to be adjusted. > > > Second...wrapping a callback's exception in a > > PregException produces > > a PregException that maps to no preg error. > > I think that is fine: Just add a new PREG_CALLBACK_ERROR that is only > emitted when PREG_THROW_ON_ERROR is set. > > > You can be holding a PregException while > > preg_last_error() and > > preg_last_error_msg() report no error at all. That is an > > exception > > whose type says a regex error happened when, by preg's own state, none > > did. > > Ah, good that you mention this, because it's not mentioned in the RFC > and I didn't check the implementation: The > preg_last_error() value > should *not* be touched when the PREG_THROW_ON_ERROR flag is set. Once > you opt into exception-based error handling, the other error handling > path should be bypassed entirely. This is consistent with how > JSON_THROW_ON_ERROR already works: https://3v4l.org/Ijt3R#veol > > <?php > > echo "Start\n"; > var_dump(json_last_error()); > echo "\n"; > > echo "Error without flag\n"; > json_decode('{'); > var_dump(json_last_error()); > echo "\n"; > > echo "Clear error\n"; > json_decode('true'); // clear error > var_dump(json_last_error()); > echo "\n"; > > echo "Error with flag\n"; > try { json_decode('{', flags: JSON_THROW_ON_ERROR); } catch > (\JsonException $e) { echo $e->getMessage(), "\n"; } > var_dump(json_last_error()); > echo "\n"; > > echo "Set different error\n"; > json_decode(str_repeat('[', 1000)); > var_dump(json_last_error()); > > echo "and check that it is not overwritten when JSON_THROW_ON_ERROR > is set\n"; > try { json_decode('{', flags: JSON_THROW_ON_ERROR); } catch > (\JsonException $e) { echo $e->getMessage(), "\n"; } > var_dump(json_last_error()); > > > This is separate from the $e->getMessage() === > > preg_last_error_msg() > > guarantee I raised before. Even setting that aside, it's incoherent on > > its own terms, because the flag is PREG_THROW_ON_ERROR and > > preg_last_error() is what an error is. > > > > So keeping that honest means a bare PregException can no > > longer > > stand for two different things at once. > > So with the above note that preg_last_error() should remain > untouched, > I believe having a single PregException for everything is fine (or > PregError + PregException, as pointed out by Robert). > > Best regards > Tim Düsterhus Hi Tim, I went looking at how some other languages answer this exact question... Python, Java and C# each have a counterpart to preg_replace_callback()...that is a regex replace that takes a callback and has its own exception to throw. And the result across the three languages is unanimous...every one of them lets the callback's exception propagate as-is. Here is each one: - Python: https://onecompiler.com/python/453rzfyge - C#: https://onecompiler.com/csharp/453rzn4dq - Java: https://onecompiler.com/java/453rwcp43 This one does not just behave this way...but the docs spell it out as the method's contract <https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/regex/Matcher.html#:~:text=Exceptions%20thrown%20by%20the%20function%20are%20relayed%20to%20the%20caller> : "Exceptions thrown by the function are relayed to the caller." Now, the other arguments made through this thread plus this one settle it toward not wrapping, then the flag keeps letting a callback's exception propagate and we are done here. However, if you still read the throwable policy as requiring the wrap, then I believe this is now a question about the policy rather than this RFC, and I would not want the RFC to hinge on this one question, so I scanned the top 4865 packages to see the cost of putting that one question aside, and here are the results: - preg_replace_callback + preg_replace_callback_array: 2,017 occurrences = 4.9% of the eight functions' total (41,413 occurrences) - the other six: 39,396 occurrences = 95.1% Anyone can check roughly the same ratio on all of public GitHub with these two searches: - https://github.com/search?q=%28%22preg_replace_callback%28%22+OR+%22preg_replace_callback_array%28%22%29+language%3APHP&type=code - https://github.com/search?q=%28%22preg_match%28%22+OR+%22preg_match_all%28%22+OR+%22preg_replace%28%22+OR+%22preg_filter%28%22+OR+%22preg_split%28%22+OR+%22preg_grep%28%22%29+language%3APHP&type=code So if we cannot agree on the callback question, I would rather move the two callback functions to future scope. Partly because I do not want to spend more weeks on a point that could end in a rejection either way, and partly because the numbers say the useful case is overwhelmingly the one with no callback in it. That is, the flag ships on the six functions where none of this arises, the two callback functions reject the flag for now, and whether their version of it wraps or propagates gets settled with the room it needs in its own follow-up after this RFC (if it passes). Regards, Osama