> Arguably this specific case is a bit debatable, but as the author of the > throwable policy RFC, I believe that it is at least violated in spirit. > The goal of the throwable policy generally, and also with regard to that > specific paragraph is to allow reliably handling groups of errors > without needing to wrap every individual statement into its own > try-catch block.
Obviously you wrote the policy and so are best placed to interpret it (and I am not a core developer / person with voting rights); however I agree with the angle Osama is coming from here - I wouldn't say this is an error that is (always) part of the same group. There wasn't any error in the call to `preg_replace_callback` itself (or any of its functionality) - the error was in a way during the processing of the result of the function. Taking your example - what if instead of `CustomException` you had `InvalidLengthException`, `IncorrectFormatException`, `NotKnownBIN`, etc. Instead of being able to catch say `InvalidLengthException` & `IncorrectFormatException` to return a validation error asking the user to check their input; `NotKnownBIN` to return that the user can not use that particular card with you, and `PregException` to note a system failure occurred then you have to catch `PregException` (or `Exception`) and then use `switch` / `match` on `$previous`. If I have understood the other example correctly, this contradicts quite significantly with the CSPRNG throwing an Exception that `RandomException` contains - as the failure is a core issue within the function call itself as opposed to logic that occurs in userland. If anything, I would argue that under the policy this should go the other way and become `PregError`: > The Error hierarchy MUST NOT be used for errors that are expected to be > thrown (and caught) during normal operation of a PHP program. In terms of the possible errors that could occur, I would expect at least `PREG_INTERNAL_ERROR`, `PREG_BAD_UTF8_ERROR` & `PREG_JIT_STACKLIMIT_ERROR` to be code errors that require a developer to need to correct their code (as my understanding of these would be that the pattern is invalid, or not quoted correctly, etc. Although `PREG_BACKTRACK_LIMIT_ERROR` & `PREG_RECURSION_LIMIT_ERROR` are more likely to occur based on user input, then the limit for both is controlled by an ini setting - so again, this likely isn't something I would say is expected to be thrown and caught during normal operation of a PHP program. The final error (`PREG_BAD_UTF8_OFFSET_ERROR`) I _think_ would still likely need a code change to fix it occurring - although I have only done a quick Google to see _when_ it may occur. I do admit that overall - my only real experience with the `preg_*` functions erroring is where the pattern itself is not valid in some way; so there may be more common use-cases that fit the exception path - but hopefully explaining why my viewpoint is to treat it as an `Error` hierarchy `Throwable` as opposed to `Exception` covers why not wrapping any throws from the userland callbacks makes sense. On Sun, Sep 6, 2026 at 12:59 PM Tim Düsterhus <[email protected]> wrote: > > Hi > > On 9/5/26 01:53, Osama Aldemeery wrote: > > Now what I would suggest instead of breaking that guarantee, is to pull > > enriching the anemic `preg_last_error_msg()` error message forward into > > this RFC instead of leaving it for later, store the real reason in the > > error state, and the exception inherits it through the very same channel, > > with the guarantee intact. > > That would also work for me. But the E_WARNING should remain when the > PREG_THROW_ON_ERROR flag is not set, because some users might rely on > the warning being emitted to turn it into an Exception themselves by > means of an error handler. > > What is important to me is that the new flag cleanly results in an > Exception and only an Exception for all possible errors, because this is > what users will expect from it. > > > On your second point, if this is a violation of a policy, then there isn't > > much to argue. I will just retract the vote and fix that. > > > > But I think I got confused here, and I would appreciate you explaining how > > that violates the policy. > > > > To make sure we're on the same ground, this is what I understood from your > > statement about wrapping exceptions thrown in user callbacks: > > > > ``` > > preg_replace_callback( > > $pattern, > > fn () => throw new CustomException(), // <- You want this wrapped in > > PregException? > > $subject, > > flags: PREG_THROW_ON_ERROR, > > ); > > ``` > > Yes. I expect a PregException where $e->getPrevious() instanceof > CustomException(). > > > If I got it right (and I suspect I did), then how does that violate the > > policy? > > A user callback isn't external functionality, is it? Because as far as I > > understand, external functionality is something the extension itself > > depends on as part of its own implementation. > > Arguably this specific case is a bit debatable, but as the author of the > throwable policy RFC, I believe that it is at least violated in spirit. > > The goal of the throwable policy generally, and also with regard to that > specific paragraph is to allow reliably handling groups of errors > without needing to wrap every individual statement into its own > try-catch block. Consider this: > > try { > $contents = get_from_api('http://example.com'); > > // sanitize credit card numbers > $contents = preg_replace_callback( > '/[0-9]{4}-[0-9]{4}-[0-9]{4}-[0-9]{4}/', > function ($matches) { > return mask_credit_card($matches[0]); > }, > $contents, > falgs: PREG_THROW_ON_ERROR, > ); > > echo $contents; > } catch (PregException $e) { > echo "Sanitization failed\n"; > } catch (HttpException $e) { > echo "Download failed\n"; > } > > I am catching the PregException to handle failures during the credit > card sanitization step. If mask_credit_card() throws its own exception > that is not wrapped, my catch blocks are insufficient and I would > instead need to write it something like this: > > try { > $contents = get_from_api('http://example.com'); > } catch (HttpException $e) { > echo "Download failed\n"; > return; > } > try { > // sanitize credit card numbers > $contents = preg_replace_callback( > '/[0-9]{4}-[0-9]{4}-[0-9]{4}-[0-9]{4}/', > function ($matches) { > return mask_credit_card($matches[0]); > }, > $contents, > falgs: PREG_THROW_ON_ERROR, > ); > } catch (Exception $e) { > echo "Sanitization failed\n"; > return; > } > echo $contents; > > To reliably handle just the exceptions that happen during sanitization > and nothing else. This is a lot of extra boilerplate code and noise. > > Now if I am still interested in the inner exception for the callback > failure, something like this would work: > > } catch (PregException $e) { > if ($e->getCode() === PregException::CALLBACK_FAILURE) { > echo "Sanitization callback failed: ", > $e->getPrevious()->getMessage(); > } else { > echo "Sanitization failed\n"; > } > } > > Because if the error code is callback failure, I know that there is a > previous Exception. So I don't lose any functionality / information. > > > I am also unaware of any functions that behave like that (wraps exceptions > > thrown in user callbacks in its own exception). > > There are a few cases where the CSPRNG (which throws RandomException on > failure) is used internally and the exception on CSPRNG failure is > wrapped. However much of the standard library predates the throwable > policy (which was accepted in May 2025; > https://wiki.php.net/rfc/extension_exceptions), that's why it doesn't > follow it. > > > In fact, the opposite is the case for one of the precedents this RFC > > follows (`json_encode()` with `JSON_THROW_ON_ERROR` - although it doesn't > > accept a user callback): https://3v4l.org/CtHYH#v8.5.10 > > Yes, that flag and JsonSerializable itself is much older than the policy. > > Best regards > Tim Düsterhus
