Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overflow exception on Numeric field when using up/down arrows to increment value #1567

Closed
phantomkingx opened this issue Jun 20, 2024 · 3 comments

Comments

@phantomkingx
Copy link
Contributor

Describe the bug
Overflow exception on Numeric field when using up/down arrows to increment value and the value overflows specific datatype.

To Reproduce
Steps to reproduce the behavior:

  1. Go to Radzen Blazor examples webpage and modify first example for Numeric field to use byte datatype for value:
    =========================================

@code{
byte value;
}

=========================================

  1. Press the 'Run' button to use the modified code in the example. The example will reload with the Numeric field set to '0'.

  2. Press the down arrow on the Numeric field and there will be a Blazor exception:

blazor.web.js:1 crit: Microsoft.AspNetCore.Components.WebAssembly.Rendering.WebAssemblyRenderer[100]
Unhandled exception rendering component: Value was either too large or too small for an unsigned byte.
System.OverflowException: Value was either too large or too small for an unsigned byte.
at System.Number.ThrowOverflowExceptionByte
at System.Decimal.ToByte(Decimal value)
at System.Convert.ToByte(Decimal value)
at System.Decimal.System.IConvertible.ToByte(IFormatProvider provider)
at System.Convert.ChangeType(Object value, Type conversionType, IFormatProvider provider)
at System.Convert.ChangeType(Object value, Type conversionType)
at Radzen.ConvertType.ChangeType(Object value, Type type)
at Radzen.Blazor.RadzenNumeric1[[System.Byte, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].ConvertFromDecimal(Nullable1 input)
at Radzen.Blazor.RadzenNumeric`1.d__12[[System.Byte, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].MoveNext()
at Microsoft.AspNetCore.Components.ComponentBase.CallStateHasChangedOnAsyncCompletion(Task task)
at Microsoft.AspNetCore.Components.RenderTree.Renderer.GetErrorHandledTask(Task taskToHandle, ComponentState owningComponentState)

Expected behavior
It would be ideal to avoid generating an exception on the page. It is possible to use Min/Max properties to avoid the exception for this particular example, however Min/Max uses the decimal datatype and an overflow exception will occur if the field is set to decimal MaxValue (79228162514264337593543950335) and the up arrow is pressed:

Additional context
Although this is somewhat of an edge case I'm not clear if there is currently a way to prevent this exception from happening.

A possible fix to this could be for the Radzen field to catch the overflow exception while ensuring no state has changed on the field. A new callback property 'Overflow' would allow for the option to handle the overflow with a user message or some other action. If we don't want this to be a breaking change incase someone expects the Blazor exception then check if the 'Overflow' property is set and rethrow the original exception if it is not.

@enchev enchev closed this as completed in 55b2e3f Jun 21, 2024
@phantomkingx
Copy link
Contributor Author

phantomkingx commented Jun 22, 2024

Thank you for addressing this so swiftly!

After viewing your solution I came up with this to cover all the native number types for .NET 7 and above. I tested this code with float, byte, int, and decimal types.

To summarize added 3 new private methods to handle step up/down for native number types in .NET >= 7.0 and a modification in UpdateValueWithStep() to uses these methods. For .NET < 7.0 or non number types process step up/down as it does currently.

#if NET7_0_OR_GREATER
        /// <summary>
        /// dynamic type wrapper for UpdateValueWithStepNumeric since TValue is not
        /// constrained to a value type
        /// </summary>
        /// <param name="value"></param>
        /// <param name="stepUp"></param>
        /// <param name="step"></param>
        /// <returns></returns>
        private dynamic UpdateValueWithStepDynamic(dynamic value, bool stepUp, decimal step)
        {
            return UpdateValueWithStepNumeric(value, stepUp, step);
        }

        /// <summary>
        /// Process the step up/down while checking for possible overflow errors
        /// and clamping to Min/Max values
        /// </summary>
        /// <typeparam name="TNum"></typeparam>
        /// <param name="value"></param>
        /// <param name="stepUp"></param>
        /// <param name="step"></param>
        /// <returns></returns>
        private TNum UpdateValueWithStepNumeric<TNum>(TNum value, bool stepUp, decimal step) 
            where TNum : struct, System.Numerics.INumber<TNum>, System.Numerics.IMinMaxValue<TNum>
        {
            var valStep = TNum.CreateSaturating(step);
            var valueToUpdate = TNum.CreateSaturating(value);

            if (stepUp && (TNum.MaxValue - valStep) < valueToUpdate) return valueToUpdate;
            if (!stepUp && (TNum.MinValue + valStep) > valueToUpdate) return valueToUpdate;

            var newValue = valueToUpdate + (stepUp ? valStep : -valStep);

            if (Max.HasValue && newValue > TNum.CreateSaturating(Max.Value) || Min.HasValue 
                && newValue < TNum.CreateSaturating(Min.Value) || object.Equals(Value, newValue))
            {
                return valueToUpdate;
            }

            return newValue;
        }

        /// <summary>
        /// Verify if value is native numeric
        /// </summary>
        /// <param name="value"></param>
        /// <returns></returns>
        private bool IsNumericType(object value) => value switch
        {
            sbyte   => true,
            byte    => true,
            short   => true,
            ushort  => true,
            int     => true,
            uint    => true,
            long    => true,
            ulong   => true,
            float   => true,
            double  => true,
            decimal => true,
            _       => false
        };
#endif

        async System.Threading.Tasks.Task UpdateValueWithStep(bool stepUp)
        {
            if (Disabled || ReadOnly)
            {
                return;
            }

            var step = string.IsNullOrEmpty(Step) || Step == "any" ? 1 : decimal.Parse(Step.Replace(",", "."), System.Globalization.CultureInfo.InvariantCulture);

#if NET7_0_OR_GREATER
            if (IsNumericType(Value))
            {
                Value = UpdateValueWithStepDynamic(Value, stepUp, step);
            }
            else
#endif
            {
                var valueToUpdate = ConvertToDecimal(Value);

                var newValue = valueToUpdate + (stepUp ? step : -step);

                if (Max.HasValue && newValue > Max.Value || Min.HasValue && newValue < Min.Value || object.Equals(Value, newValue))
                {
                    return;
                }

                if ((typeof(TValue) == typeof(byte) || typeof(TValue) == typeof(byte?)) && (newValue < 0 || newValue > 255))
                {
                    return;
                }

                Value = ConvertFromDecimal(newValue);
            }

            await ValueChanged.InvokeAsync(Value);
            if (FieldIdentifier.FieldName != null) { EditContext?.NotifyFieldChanged(FieldIdentifier); }
            await Change.InvokeAsync(Value);

            StateHasChanged();
        }

@enchev
Copy link
Collaborator

enchev commented Jun 23, 2024

Thanks @phantomkingx! You can submit pull request!

@phantomkingx
Copy link
Contributor Author

Yes created PR #1570

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants