Implement optional custom error messages for *all* validation parameter attributes (not just ValidateScript and ValidatePattern)

Staffan Gustafsson (@powercode)

^{This is a follow-up to #3630.

Related: #3765}

Thanks to Staffan Gustafsson (@powercode)'s efforts in #2728, , we now have an optional ErrorMessage property in the ValidatePattern and ValidateScript parameter validation attributes. However, all validation attributes (Validation*; all that derive from System.Automation.Management.ValidateArgumentsAttribute) should support a custom error message:

Even for the simplest validations it can sometimes be helpful to provide domain-specific guidance rather than reporting a purely technical violation (see example below).
Consistent support eliminates the burden of having to remember which specific attributes support the property (though IntelliSense may ease that pain).

Example

function foo {
  param(
  [ValidateCount(2, [int]::maxvalue, ErrorMessage='This bar has a 2-drink minimum.')]
  [string] $bar
  )
  $bar
}

foo -bar 'mint julep'

Desired behavior

foo : Cannot validate argument on parameter 'bar'. This bar has a 2-drink minimum.
...

As in the implementation for ValidatePattern and ValidateScript, placeholder {0} would represent the value passed by the user, and {1}, ... the validation-attribute arguments.

This idea was discussed and rejected in that PR, e.g. #2728 (comment)

Jason Shirk (@lzybkr)

Jason Shirk (@lzybkr):

(Your link label suggests that it links to a specific comment in the thread, but it doesn't.)

Reading through that thread, without understanding all the subtleties, I see a potential rejection argument based on technical inconvenience:

I think your suggestion was proposed and I'm not in favor of it because it provides a false promise - a validation attribute may ignore the property.

After further thought - there might be a way for the engine to check if an error message uses the ErrorMessage property in the exception's message - and if it doesn't - replace the error.

I think this might be reasonable to implement, but it feels a little clunky. Because Exception.Message is read-only - we'd need to create a new exception, likely using the original one as the InnerException.

Is this what you're referring to?

Either way, for future reference, could you please summarize the reason for rejecting this proposal here, given that it makes the case for all validation attributes supporting a custom error message in a more focused fashion?

Aside from what subset of validation attributes should support custom error messages: what are your thoughts on supporting localization? Should I create a separate issue?

Michael Klement (@mklement0)

I still think we should be more common here with custom message.

Michael Klement (@mklement0) I believe that we should not mix "custom message" and "message localization". Localizing messages for scripts looks a bit excessive complication.

If ErrorMessage is available to all validation attributes, we are forced to choose between:

Inconsistent support - not every implementation of the attribute will utilize the message
Consistent support via hacky means - basically the engine would be forced to do unnatural things to ensure the message is used

Neither option is ideal. Existing validation attributes that know nothing about this new property may not be updated, and script authors may not test their use of ErrorMessage, so they may not realize it's at best a comment.

On the other hand, if the PowerShell engine is forced to wrap an exception when it detects the ErrorMessage is not used, the engine is making a choice to trust the user of the attribute over the author of the attribute. This might be a reasonable assumption, but it might not. But worse, to use the custom error message, PowerShell would be forced to create a new exception, and whenever we wrap exceptions, we are creating an inconsistent experience because sometimes you need to unwrap (when ErrorMessage is used) and sometimes you don't.

Also, I do think localization is a distinct issue.

Ilya (@iSazonov)

Re localization: Makes sense - please see #3765.

Ilya (@iSazonov): I know that localization is a feature not used by many, but we should support it comprehensively.

Jason Shirk (@lzybkr)

Jason Shirk (@lzybkr): Thanks for the summary.

Consistent support via hacky means - basically the engine would be forced to do unnatural things to ensure the message is used

I can see how that's undesirable.

Inconsistent support - not every implementation of the attribute will utilize the message

Are you referring to third-party implementations of attributes?

Simply ensuring that all validation attributes that ship with PowerShell support custom error messages, combined with documenting the recommendation that third-party implementers respect them too seems like a reasonable compromise to me.

Without the hack, users are already left guessing: namely, as to what subset of the shipping-with-PowerShell attributes happen to implement custom error messages, and what governed membership in that subset.

Yes, third party implementations, and I suppose future first-party implementations where somebody forgets about this property.

I believe it's the engine's responsibility to provide good error messages everywhere, and only when that's not possible should we allow customization.

Another example (copied because GitHub can't seem to link correctly) :

With a custom error message, it's possible for a cmdlet author to provide a worse error message than the default, especially without localized messages.

For a few of the attributes, I can't convince myself it's worth it. And there are minor costs - an extra pointer that is never used isn't free - it requires more memory and extra time in garbage collection checking if the (usually null) value needs collecting.

ValidateNotNullOrEmpty stands out as one example where I wouldn't bother. ValidateDrive is also probably not worth it.

And last - I can see a custom error message being useful on ValidateRange, e.g. to say the value should be a positive value or whatever, but if we go that route, we need localization support similar to HelpMessageBaseName and HelpMessageResourceId in ParameterAttribute.

So using the ValidateRange example, I would rather we add ValidatePositive and ValidateNonNegative attributes so that nobody has to worry about localization and is not tempted to use ValidateRange with the ErrorMessage property.

As for guessing - Intellisense does a good job completing property names in attributes, so I don't think it's a big problem.

Thanks for that. Looking at the set of built-in attributes more closely, I now see how there's not much benefit in allowing custom messages for the remaining ones (doing so would also be complicated by several attributes reporting one of several messages).

In that vein:

I hadn't noticed that ValidateSetAttribute was also given an ErrorMessage property - it seems like a prime candidate for not needing that.
Conversely,
- ValidateLength deserves an analogous makeover to ValidateCount's (ValidateCount parameter-validation attribute error messages are misleading and inflexible #3585):
  - distinguish only 2 cases: min. and max. length being equal, and being outside the range
  - the current messages are quite pleonastic (I'm in the mood for big words) and prescriptive; e.g.: The character length of the 3 argument is too long. Shorten the character length of the argument so it is fewer than or equal to "2" characters, and then try the command again
- Similarly, ValidateRange should probably have only a single (built-in) outside-the-range error message (a min == max case doesn't make sense here).

nobody has to worry about localization

Given that we'll have (at least) 2 attributes that do support custom error messages - ValidateScriptAttribute and ValidatePatternAttribute - isn't the localization can of worms / gusanos / Würmer / vers already open?

P.S.: The link to the specific comment in the other thread worked this time.

I approved the custom message on ValidateSet because sometimes the set is so large that a custom message might be better.

For completeness, we should support localization of the custom error message, but I didn't raise that issue because:

we don't currently have any localization outside of official Windows PowerShell
a custom error message is very likely better than any localized message for the generic message for the attributes where we allow the custom error message

This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes.

Navigation Menu

Repository navigation

Implement optional custom error messages for all validation parameter attributes (not just ValidateScript and ValidatePattern) #3745

Example

Desired behavior

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Implement optional custom error messages for *all* validation parameter attributes (not just ValidateScript and ValidatePattern) #3745

Description

Example

Desired behavior

Activity

lzybkr commented on May 10, 2017

mklement0 commented on May 10, 2017

iSazonov commented on May 10, 2017

lzybkr commented on May 10, 2017

lzybkr commented on May 10, 2017

mklement0 commented on May 11, 2017

mklement0 commented on May 11, 2017

lzybkr commented on May 11, 2017

mklement0 commented on May 11, 2017

lzybkr commented on May 12, 2017

microsoft-github-policy-service commented on Nov 16, 2023

microsoft-github-policy-service commented on Nov 16, 2023

microsoft-github-policy-service commented on Nov 16, 2023

microsoft-github-policy-service commented on Nov 23, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Implement optional custom error messages for all validation parameter attributes (not just ValidateScript and ValidatePattern) #3745