ParameterView unchecked type casts when splatting provides unhelpful error messages
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 281
Description
## Summary
When setting component parameters directly compile time analysis will find mismatched types and provide a helpful error message pointing to the exact position of the problem with the type cast failing. If instead a developer splats attributes onto the component then a mismatched type will only be caught at runtime during `ParameterView.HasParameterChanged()` which results in an unhelpful error message stating that types could not be converted (e.g. could not convert from Int32 to Double) but does not provide the name of the parameter or calling component.
## Motivation and goals
Vague error messages at runtime are confusing and upsetting to the best developers, adding a type check and providing some more specific information will aid in tracking down specific attributes causing issues without having to comb through every component and splat in use on a page.
## In scope
Developer splats attributes onto a component, the type of an attribute does not match: the developer should receive a specific runtime error which provides the name of the parameter and calling component with the mismatch.
## Out of scope
Static analysis for attribute splat type checking. While that would be a boon, I think it is best to focus on helpful error messaging first and preventative measures later.
## Risks / unknowns
If an exception other than InvalidCastException is thrown from within ParameterView this could cause issues for developers who are catching the specific exception, but I find it an unlikely scenario given the type of issue.
## Examples
I ran into this today using a MudTextField component from MudBlazor
```home.razor
var debounceSplat = new Dictionary{ {"DebounceInterval", 500} };
//This splat uses the wrong type, DebounceInterval expects an int32.
//Works, implicit cast from int to double
//Results in expected compile time error message
//Results in vague exception produced from ParameterView
```
Using the debounceSplat as above should result in an error that looks like
```
Unhandled exception rendering component: Unable to cast object of type 'System.Int32' to type 'System.Double'.
System.InvalidCastException: Unable to cast object of type 'System.Int32' to type 'System.Double'.
at Microsoft.AspNetCore.Components.ParameterView.TryGetValue[TValue](String parameterName, TValue& result)
at MudBlazor.ParameterViewExtensions.HasParameterChanged[T](ParameterView parameters, String parameterName, T parameterValue, T& value, IEqualityComparer`1 comparer)
at MudBlazor.State.ParameterStateInternal`1.HasParameterChanged(ParameterView parameters)
at MudBlazor.State.ParameterContainer.CollectChangedHandlers(ParameterView parameters)
at MudBlazor.State.ParameterContainer.SetParametersWithHandlersAsync(Func`2 baseSetParametersAsync, ParameterView parameters)
at MudBlazor.MudBaseInput`1.SetParametersAsync(ParameterView parameters)
```
Even just including the name of the parameter in the error message would go a long way towards helping developers solve this problem in the wild.
Contributor guide
Research direction
Start by reading ParameterView.HasParameterChanged() and the ParameterView.TryGetValue() entry point referenced in the report, then trace how an InvalidCastException is produced for splatted attributes. Add coverage for a mismatched splatted parameter and verify that the runtime error includes the parameter and calling component names without changing static-analysis behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100