On Mon, Sep 7, 2026 at 3:32 PM Tim Düsterhus <[email protected]> wrote:
>
> Hi
>
> On 2026-09-07 13:58, Robert Humphries wrote:
> >> 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
>
> Yes, I agree that this case is not entirely clear-cut - and it's good
> we're having this discussion now.
>
> > 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.
>
> I think there might be a misunderstanding based on how you phrased that
> paragraph. To provide a more specific example:
>
> Consider I have a session implementation that uses Redis as its session
> storage backend. Session IDs need to be created using secure randomness,
> i.e. using the CSPRNG. Both the Redis backend and the CSPRNG can
> theoretically fail. As a user when create a new session I want to be
> able to just catch (SessionInitializedFailedException) and not care
> about whether the CSPRNG or the Redis connection failed, and I might not
> even know if it's Redis, Memcache, a File System or a MySQL database.
> Thus any underlying issues must be wrapped into a session-specific
> exception.
>
> preg_replace_callback() is different in that I explicitly pass in a
> callback and thus I'm technically in full control over the code that is
> being executed and I can theoretically know what exceptions could
> possibly be thrown and might intentionally want to handle them
> explicitly. On the other hand, failing to execute the callback means
> that the replacing operation failed, no further callbacks will be called
> and preg_replace_callback() will not return anything - and that is a
> “running this regex failed” a.k.a. PregException situation to me.
>
> > 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.
>
> This is a good point. I agree that things like pattern compilation
> failures should be a PregError, since this is a clear programmer error
> and regular expressions are not supposed to be untrusted inputs. For the
> error error situations I would need to check as well if they are
> expected during regular operation or not. The backtrack or recursion
> limits I can see being caught intentionally to provide better error
> messages to a user (thus PregException).
>
> Best regards
> Tim Düsterhus
Hi Tim,
A couple of updates after digging into this further...
1. I found a case that makes the `$e->getMessage() ===
preg_last_error_msg()` guarantee I leaned on not really hold in general.
A `PregException` freezes its message and code when it is thrown, but
`preg_last_error_msg()` reads the per-request global.
And so the two agree only while nothing runs between the throw and the read.
A callback that makes its own flagged call breaks that...when the inner
call throws, the outer operation bails and overwrites the global with
`INTERNAL_ERROR` (why it lands on that code is point 2) before you read it.
For example:
```
try {
preg_replace_callback(
'/\w/',
function ($m) {
preg_match('//u', "\xff", $inner, PREG_THROW_ON_ERROR); //
the inner call throws
return 'Y';
},
'a',
flags: PREG_THROW_ON_ERROR,
);
} catch (\PregException $e) {
var_dump($e->getMessage()); // "Malformed UTF-8 characters, ..."
frozen in the exception
var_dump(preg_last_error_msg()); // "Internal error" the global,
already overwritten
var_dump($e->getCode()); // 4 PREG_BAD_UTF8_ERROR
var_dump(preg_last_error()); // 1 PREG_INTERNAL_ERROR
}
```
So the equality is a property of a single flagged call, not an
invariant...and the exception is a faithful snapshot of its own call, while
`preg_last_error_msg()` is a global the next call moves.
I should have stated it that narrowly.
2. I was also wrong about the callback case.
When a user callback throws, PHP does set `preg_last_error()` and
`preg_last_error_msg()`.
They come back as `PREG_INTERNAL_ERROR` and "Internal error", not "No
error", so my earlier claim that a callback throw leaves the error state
untouched was incorrect.
I dug in the C code to see how that works, and how that value gets set
changes what it means.
When the callback throws, the replacement bails out (
https://github.com/php/php-src/blob/82a15338e298142f52854b64d803695d4e5252df/ext/pcre/php_pcre.c#L1960),
and the `error:` path calls `pcre_handle_exec_error(count)` (
https://github.com/php/php-src/blob/82a15338e298142f52854b64d803695d4e5252df/ext/pcre/php_pcre.c#L2032-L2033
).
At that point `count` is the successful match count, not a PCRE error code,
and `pcre_handle_exec_error()` has no case for a non-negative value, so it
falls through to its default and returns `PHP_PCRE_INTERNAL_ERROR` (
https://github.com/php/php-src/blob/82a15338e298142f52854b64d803695d4e5252df/ext/pcre/php_pcre.c#L132
).
The engine matched fine, but the `"Internal error"` is a fallback the
bail-out path leaves behind, not a diagnosis of anything PCRE did wrong.
That means there is no real message for this case...
A genuine PCRE error carries a specific reason (`"Malformed UTF-8 ..."`,
`"Backtrack limit exhausted"`, and so on...)...
A callback throw only ever yields the generic `"Internal error"`, because
nothing in PCRE actually failed.
The real information is the exception the callback threw, which carries its
own message and type.
So I would say this case is unlike the PCRE errors in a concrete way...that
is for them the error state describes the failure, but here it does not.
To me that leans (though it does not settle) toward letting the callback's
exception propagate rather than wrapping it.
Wrapping would turn a specific userland exception into a `PregException`
whose own message can only be `"Internal error"`.
Whether that lean is enough, or whether not wrapping here is still a
violation of the throwable policy, I think is your call as its author. I am
only laying out what the code does.
With that said, I think the things we have so far that make the case for
letting the callback's exception propagate are:
- A callback throwing is not a PCRE error. The engine matched, and
userland threw. The `"Internal error"` it records is the fallback above,
not a diagnosis.
- There is no real message to deliver. The genuine message and type are
in the callback's own exception. A `PregException` here would carry only
`"Internal error"`.
- Wrapping destroys catch-by-type. Domain exceptions from the callback
all collapse into `PregException`, so callers can no longer catch them by
type and have to inspect `->getPrevious()`.
- It is the more flexible default. A caller who wants a single catch can
throw a `PregException` from their own callback. A caller who wants their
own types back under wrapping cannot get them without catch-and-rethrow.
Now whether any of these is a knockout on its own is probably
arguable...But I think together they justify letting the callback's
exception propagate.
But of course it turns on how the throwable policy should apply, so I need
your judgement here.
Thanks,
Osama
