Add Method to UnhandledMatchError to get the Unhandled Value

[donatj]

Description

When catching a failed match it would be very handy if the UnhandledMatchError had a method such as getUnmatchedValue() : mixed that contained the actual unmatched value.

This could allow more graceful handling of failed match handlers.

This would allow as a very basic example:

function getTypeName( mixed $value ) : string {
    return match (gettype($value)) {
        'integer', 'double' => 'number',
        'string'            => 'string',
        'NULL'              => 'null',
    };
}

try {
    getTypeName($item);
} catch(\UnhandledMatchError $e) {
    $log->info("Received unhandled type: " . $e->getUnmatchedValue());
}

I'm sure there is more involved handling you could do with it as well.

@derickr advised me to open a ticket for this idea

[iluuu1994]

Hi @donatj! Thanks for the suggestion. I'm not personally in favor, because 1. I don't believe you should generally catch \Errors, which makes the value uninteresting, and 2. we don't do this for any other ValueErrors. The value itself is also already embedded in the message. But let's see what others think.

[donatj]

Thanks you for the feedback. I do, however, disagree that you shouldn't catch errors.

Why would PHP move from warnings to errors to if not to make them more easily catchable/handleable? Being able to catch them is particularly useful I find in reflection/inspection - especially in cases where you code is running someone else code that might not be perfectly formed. Providing as much info as possible helps others fix issues more efficiently.

Catching errors allows for graceful error handling, and adding helpful detail only serves makes it more useful.

For ValueError, I could actually argue having the specific parameter that failed would also be very useful, though probably out of scope here.

Open to more thoughts on this.

[iluuu1994]

Why would PHP move from warnings to errors to if not to make them more easily catchable/handleable?

Fatal errors are very harsh, and warnings are error prone because they don't terminate control flow. With exceptions you can handle generic errors gracefully in your front controller and display an error message accordingly. I don't think catching specific errors makes much sense, because if you know your code can throw errors, you should fix the code. Of course, you can't know about all errors in advance, but then you also don't know about its type (is it a UnhandledMatchError, ValueError, or whatnot).

Note that this reasoning is exactly what we use to determine whether something internally should throw an error or an exception: Do we expect users to want to catch them, or is the problem unrecoverable? For the former, we use exceptions, and for the latter we use errors.

Do you have a use case other than logging? Because then the value is already visible in the error message (if it is inspectable, that is). If providing more information is the goal, maybe this is better solved by improving error messages in all places, so all users can benefit.

[Girgias]

I agree with @iluuu1994 here, I'm not really in favour of adding such capabilities.

The Error hierarchy is for programming errors that can be prevented at runtime, the advantage of using the exception mechanism instead of bailout is that destructors are run, finally blocks are executed, the stack is unwound properly, etc.

Some more rationale about this can be found in the original RFC to convert bailouts to Exceptions. [1]

As one of the main people that converted E_WARNINGs to ValueErrors for PHP 8.0 was the added return type clean-up we could do.
Indeed, a lot of functions/methods only returned false/null for programming level errors, and so now you don't need to branch on an unlikely unexpected return value. However, many E_WARNINGs remain as the issues they point to cannot be prevented in advance as they depend on the environment or runtime conditions.

[donatj]

Well if no one is interested, we can probably close this.

[iluuu1994]

Do you have a use case other than logging?

If logging is the only use-case, feel free to close this issue 🙂

[github-actions]

No feedback was provided. The issue is being suspended because we assume that you are no longer experiencing the problem. If this is not the case and you are able to provide the information that was requested earlier, please do so. Thank you.

[WM-Programator-3]

Hello, im not sure if it is appropiate to join this discussion now as the issue seems to be closed, but seems like some counterarguments against this feature are based on not completely valid information?

Hi @donatj! Thanks for the suggestion. I'm not personally in favor, because 1. I don't believe you should generally catch \Errors, which makes the value uninteresting, and 2. we don't do this for any other ValueErrors. The value itself is also already embedded in the message. But let's see what others think.

At the moment it is not possible to ge the value in some cases:

$value = '12345678901234567890';
	
match ($value) {
    '' => null
};

Message:
Unhandled match case '123456789012345...'

So currently you would do what? Catch to get the value?

try {
    $value = '12345678901234567890';
    
    match ($value) {
        '' => null
    };
    
} catch (UnhandledMatchError $e) {
    throw new Exception("Unhandled match case '".$value."'");
}

Message:
Unhandled match case '12345678901234567890'

I agree that it is best to throw own, more informative errors like this:

$value = '12345678901234567890';

if (!in_array($value, ['a', 'b', 'c'], true)) {
	throw new InvalidArgumentException("Invalid value '".$value."'. Supported values: a, b, c.");
}

match ($value) {
    'a' => 1,
    'b' => 2,
    'c' => 3
};

Message:
Invalid value '12345678901234567890'. Supported values: 'a', 'b', 'c'.

But the errors are in place because errors still happen because reasons, and it is not always our code, or sometimes even a code we have control over, that does.
In those real-life scenarios when one would like to get the value, lets say in global error handler, the proposal makes sense.
When things go wrong, the UnhandledMatchError sometimes does not contain all necessary information and it can be very tedious to find out what value specifically it was and thus where it came from and what needs to be fixed.

So back to the original arguments:

1. I don't believe you should generally catch \Errors, which makes the value uninteresting

My counter argument here is:

Saying that a value is not interesting because catching is bad makes as much sense as saying that vegetables are not healthy, because you don't like them.

You should catch errors, if the value is "interesting" (important) and catching is the only way to get it.
Or are you gonna tell your boss or customers that you could do something, it is easy, reliable, but it goes against good practices, which is reason why they got inferior product?
Nonsense.

2. we don't do this for any other ValueErrors. The value itself is also already embedded in the message.

No, it is not always there, at least not whole. And if your argument is that this is how you do it in other places, then other places are probably a problem aswell.

If providing more information is the goal, maybe this is better solved by improving error messages in all places, so all users can benefit.

Yes please.

[donatj]

I think @WM-Programator-3 's example of handling failed matches outside your projects purview is a great example of a use case

[iluuu1994]

But the errors are in place because errors still happen because reasons, and it is not always our code, or sometimes even a code we have control over, that does.

I still don't understand. If you need to find out why the match has failed, the value passed to the match is already visible in the error message. If you need to understand what values are being accepted, most frameworks will print excerpts of the stack trace, with the first frame being the match in this case. Adding the default value to the exception would not help for this case anyway.

In those real-life scenarios when one would like to get the value, lets say in global error handler, the proposal makes sense.

But does it? Catching a random UnhandledMatchError with absolutely no context does not seem useful. What would a generic error handler even do with a UnhandledMatchError value? It's the equivalent of catching any InvalidArgumentException. What would you even do with this? How do you differentiate between other, buggy match expressions and the actual expression you want to catch? If there is one specific expression, why not handle it gracefully there?

Anyway, this issue has already been discussed too much on GH. Please move it to the mailing list. If an RFC is created and accepted, I can't stop you from adding it to core. But I personally don't see the point.

[WM-Programator-3]

I still don't understand. If you need to find out why the match has failed, the value passed to the match is already visible in the error message.

The value is not present in the error message whole, if the value is longer than 15 characters.

Here is the example again:

$value = '12345678901234567890';
	
match ($value) {
    '' => null
};

Message:
Unhandled match case '123456789012345...'

It is not enough if you are working with longer values.

For example, Base.com does not have ID's for delivery, they only give you the delivery name.
If you want to convert the delivery name to your own internal ID, you end up with this:

$result = match($input) {
    'paczkomat 24/7'             => '...',
    'paczkomat 24/7 pobranie'    => '...',
    'paczkomaty 24/7'            => '...',
    'paczkomaty 24/7 pobranie'   => '...',
    'paczkomat inpost'           => '...',
    'paczkomat inpost pobranie'  => '...',
    'paczkomaty inpost'          => '...',
    'paczkomaty inpost pobranie' => '...',
    'std'                        => '...',
    'std pobranie'               => '...',
    'kurier fedex'               => '...',
    'kurier fedex pobranie'      => '...',
    'orlen paczka'               => '...',
    'orlen paczka pobranie'      => '...',
};

The value in error message would be this:

'paczkomat 24/7'
'paczkomat 24/7 ...'
'paczkomaty 24/7'
'paczkomaty 24/7...'
'paczkomat inpos...'
'paczkomat inpos...'
'paczkomaty inpo...'
'paczkomaty inpo...'
'std'
'std pobranie'
'kurier fedex'
'kurier fedex po...'
'orlen paczka'
'orlen paczka po...'

As you can see, when working with text, you would very often not get the whole value in error message.

But does it? Catching a random UnhandledMatchError with absolutely no context does not seem useful. What would a generic error handler even do with a UnhandledMatchError value? It's the equivalent of catching any InvalidArgumentException. What would you even do with this? How do you differentiate between other, buggy match expressions and the actual expression you want to catch? If there is one specific expression, why not handle it gracefully there?

I think you are still working with the assumption that the whole value is visible in the error message.
If it is not, as shown above, catching this exception specifically would serve the purpose of doing something extra to get the actual value - to enable developers to find and/or fix a problem in fraction of time it would take otherwise, where otherwise means now, because we get an error and a teaser to the value, but not the exact value.

I believe it would be more like catching a SoapFault to get the detail in addition to the error message.
It is very helpful even in global error handler without any context, as it enables devs to more easily handle problems, no matter where in the application it happened.

Again, working with the fact that the value is actually not contained (not whole) in the error message, and you have to do something extra to get it, something that you would not do with other exceptions.

Anyway, this issue has already been discussed too much on GH.

Sorry, but i am not sure what GH means.
If this issue has been discussed somewhere else, can you send me a link or something?
If you mean this place, then till now, you were not even able to understand what the issue we are talking about is, so it has been hardly properly discussed.

[WM-Programator-3]

Sorry, i just realized GH probably means GitHub.
Either way, if it has been discussed only in this issue so far, then it has not been discussed properly, as one side still has no idea what the conversation even is about it seems.

[Girgias]

GitHub is not where we discuss such topics. If you feel so strongly about it, email the PHP internals mailing list.

[iluuu1994]

Yes, it's true that the value may be clipped. In that case, you are right that the message is insufficient to diagnose the error. However, this is easily solved with a var_dump or a breakpoint in your debugger. And yes, you can absolutely temporarily modify a file that doesn't belong to you. This is one of the great upsides of PHP + Composer. Even if the value was stored in the exception, you would still have to modify your code to catch and print it.

then it has not been discussed properly, as one side still has no idea what the conversation even is about it seems.

I never said stop discussing this suggestion, but discuss it on the mailing list where it belongs. PHP is community driven, and the official medium for discussions is the mailing list. GitHub is used for bug reports and simple, uncontroversial feature requests. When there are objections, they should be discussed with the community.

[WM-Programator-3]

Thanks for the info, i will switch to the mailing list now.