On Sun, Sep 6, 2026 at 2: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

Hi Tim,

Thank you for the elaboration.

The first thing that came to my mind reading your example is that
without wrapping, an extra `catch` block would be enough:

```
try {
    $contents = get_from_api('http://example.com');

    $contents = 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,
        flags: PREG_THROW_ON_ERROR,
    );

    echo $contents;
} catch (MaskException $e) {
    echo "Sanitization failed\n";
} catch (PregException $e) {
    echo "Regex failed\n";
} catch (HttpException $e) {
    echo "Download failed\n";
}
```

The caveat of course is that this only works when you know the
exception a callback can throw...which you don't always control.

But since you've made clear this is a violation of the throwable
policy, I've retracted the vote until we get it resolved.

Following the idea of wrapping the callback's exception, I see two problems...
One is a footgun I'd want explicitly addressed.
The other, I'm afraid, forces an exception hierarchy in place of a
single `PregException`.

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
}
```

Second...wrapping a callback's exception in a `PregException` produces
a `PregException` that maps to no preg error.
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.
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 it has to split into a `PregPcreException` (or a better name)
carrying the real preg error and mirroring `preg_last_error()`...and
`PregCallbackException` wrapping the callback's exception. And then to
satisfy the single catch both would need to extend a shared
`PregException` parent.
That's a whole hierarchy to carry a case an extra `catch` would have handled.

So the cost of wrapping comes out as a silent footgun plus a
three-class hierarchy, for what one more `catch` could do.
Both look intrinsic to wrapping, though. The footgun is the flip side
of the single catch you want, and the hierarchy is what keeps that
catch honest, so neither can really be designed away.

That is if I didn't miss anything along the way...

So my question now is: is that price worth paying over the extra
`catch`...or is the extra `catch` acceptable after all?
That's where I'd hope your "Arguably this specific case is a bit
debatable" leaves some room.

In any case...I'll be working on how the `preg_last_error_msg()`
change impacts the RFC, and meanwhile looking for opinions on the
wrapping points I've raised above.

Regards,
Osama

Reply via email to