ArgumentNullException in GetValidationErrors

[Vadim]

Hello

Do I get it right, that when I receive following exception, that means that somewhere my ```INotifyDataErrorInfo.GetErrors``` returns null instead of empty list?

```

System.ArgumentNullException: Value cannot be null.
Parameter name: collection
   at System.ThrowHelper.ThrowArgumentNullException(ExceptionArgument argument)
   at System.Collections.Generic.List`1.InsertRange(Int32 index, IEnumerable`1 collection)
   at Telerik.Windows.Controls.GridViewBoundColumnBase.GetDataErrorValidationErrors(Object dataItem, String propertyName, GridViewValidationType gridViewValidationType)
   at Telerik.Windows.Controls.GridViewBoundColumnBase.GetValidationErrors(Object dataItem, String propertyName)
   at Telerik.Windows.Controls.GridView.GridViewCell.GetDataErrors()
   at Telerik.Windows.Controls.GridView.GridViewCell.UpdateIsValidState()
   at Telerik.Windows.Controls.GridView.GridViewDataControl.Telerik.Windows.Data.IWeakEventListener<System.ComponentModel.DataErrorsChangedEventArgs>.ReceiveWeakEvent(Object sender, DataErrorsChangedEventArgs args)
   at System.EventHandler`1.Invoke(Object sender, TEventArgs e)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)

```

[Vladimir Stoyanov]

Hello Vadim,

Thank you for the shared stack trace. 

I investigated the related code, however it seems that the "null" scenario is handled by the RadGridView. Here is the GetDataErrorValidationErrors implementation:

internal static IEnumerable<string> GetDataErrorValidationErrors(object dataItem, string propertyName, GridViewValidationType gridViewValidationType)
		{
			List<string> errors = new List<string>();

			if (gridViewValidationType.IsGridViewValidationTypeSet(GridViewValidationType.IDataErrorInfo))
			{
				IDataErrorInfo dataErrorInfo = dataItem as IDataErrorInfo;
				if (dataErrorInfo != null && dataErrorInfo.GetErrorsForProperty(propertyName) != null)
				{
					errors.AddRange(dataErrorInfo.GetErrorsForProperty(propertyName));
				}
			}
			if (gridViewValidationType.IsGridViewValidationTypeSet(GridViewValidationType.INotifyDataErrorInfo))
			{
				INotifyDataErrorInfo notifyDataErrorInfo = dataItem as INotifyDataErrorInfo;
				if (notifyDataErrorInfo != null && notifyDataErrorInfo.GetErrorsForProperty(propertyName) != null)
				{
					errors.AddRange(notifyDataErrorInfo.GetErrorsForProperty(propertyName));
				}
			}
			return errors;
		}

and the GetErrorsForProperty implementation:

public static IEnumerable<string> GetErrorsForProperty(this INotifyDataErrorInfo notifyDataErrorInfo, string propertyName)
		{
			var errorsForProperty = notifyDataErrorInfo.GetErrors(propertyName);

			if (errorsForProperty == null)
			{
				return null;
			}

			List<string> result = new List<string>();

			foreach (var error in errorsForProperty)
			{
				result.Add(error.ToString());
			}

			return result.Count > 0 ? result : null;
		} 

You can observe that there is a check for not null and only if it passes, the AddRange method is called. 

That said, I am not sure what might be the cause for the mentioned exception. If you find it possible, you can modify the ValidationINotifyDataErrorInfo SDK example in order to demonstrate the error and send it back. This way I will be able to investigate it on my end and better assist you. 

I am looking forward to your reply.

Regards,
Vladimir Stoyanov
Progress Telerik

[Vadim]

Hello Vladimir,

Apologies for delay. It seems avoiding null, when returning errors in GetError property has resolved my issue. Cannot say for sure as it's not very reproducible. On the other hand in the code, you have kindly provided, there is a race condition.

Checking dataErrorInfo.GetErrorsForProperty(propertyName) for null and calling it again may fail when background thread updates errors.

To fix it you would need to rewrite it as follows:

if (dataItem is IDataErrorInfo dataErrorInfo && dataERrorInfo.GetErrorsForPropert(propName) is IEnumerable<blahblah> errs)
{
    errors.AddRange(errs);
}

[Vadim]

And now I realized that even if I return empty errors list instead of null the race condition can still get triggered, because GetErrorsForProperty returns null for empty result. Design guidelines recommend to return empty collection wherever a collection type is return, never null.

https://stackoverflow.com/a/1970058/1397782

[Vladimir Stoyanov]

Hello Vadim,

Thank you for the updates. 

May I ask you to elaborate a bit on your scenario? Are you updating the errors from a background thread on your end? If that is the case, can you share some sample code or a project (you can attach one in a new support ticket, since projects cannot be added to forum posts)? This will hopefully allow me to get a better understanding of the scenario and try to replicate the exception. Note, that we prefer to have to have a good idea of the exact scenario with a reproduction project before introducing changes in our source code.

Thank you in advance for any help you can provide. 

Regards,
Vladimir Stoyanov
Progress Telerik

[Vadim]

Hello Vladimir,

Yes, I am updating errors from an async Task (so from threadpool). Regarding sample code, it's does not look promising, as I may spend hours to try to tune system to kick the race condition, that will only be reproducible on my machine, but not on yours.

Yet, I must insist, the race condition can be seen with a naked eye right in the code samples you have kindly provided on 17th Sep 2020. See my commends from 1.10.2020.

[Vladimir Stoyanov]

Hello Vadim,

Thank you for the update. 

I discussed the scenario with our developers and we decided to make the proposed improvement to avoid calling the GetErrorsForProperty method twice. We have introduced the change and it will be available with our next internal build, which will be released in the beginning of next week. I have also added some telerik points to your account as a thank you for bringing this to our attention. 

Of course, do not hesitate to contact us again, if you have any other questions.

Regards,
Vladimir Stoyanov
Progress Telerik