Re: [RFC] [VOTE] PREG_THROW_ON_ERROR

> 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 <tim@bastelstu.be> 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