dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.06k stars 2.03k forks source link

Could not find a copier for type System.Linq.EmptyPartition<T> #8368

Closed EugeneKrapivin closed 1 year ago

EugeneKrapivin commented 1 year ago

Having a serialization issue when trying to return an empty enumerable

this is the model

    [GenerateSerializer]
    public class Workspace
    {
        [Id(0)]
        public Guid Id { get; set; }

        [Id(1)]
        public string Name { get; set; }

        [Id(2)]
        public string? Description { get; set; }

        [Id(3)]
        public bool IsTesting { get; set; }

        [Id(4)]
        public IEnumerable<string> Licenses { get; set; } = Enumerable.Empty<string>();

        [Id(5)]
        public IEnumerable<BusinessUnit> BusinessUnits { get; set; } = Enumerable.Empty<BusinessUnit>();
    }

When returning a new instance of this model, without initializing the BusinessUnits property I'm getting:

Orleans.Serialization.CodecNotFoundException: Could not find a copier for type System.Linq.EmptyPartition`1[ConfigurationService.Contracts.BusinessUnit].
   at void Orleans.Serialization.Serializers.CodecProvider.ThrowCopierNotFound(Type type) in /_/src/Orleans.Serialization/Serializers/CodecProvider.cs:line 666
   at IDeepCopier Orleans.Serialization.Serializers.CodecProvider.GetDeepCopier(Type fieldType) in /_/src/Orleans.Serialization/Serializers/CodecProvider.cs:line 326
   at T Orleans.Serialization.Cloning.CopyContext.DeepCopy<T>(T value) in /_/src/Orleans.Serialization/Cloning/IDeepCopier.cs:line 263
   at object Orleans.Serialization.Codecs.ObjectCopier.Orleans.Serialization.Cloning.IDeepCopier.DeepCopy(object input, CopyContext context) in /_/src/Orleans.Serialization/Codecs/ObjectCodec.cs:line 133
   at T Orleans.Serialization.Cloning.UntypedCopierWrapper<T>.DeepCopy(T original, CopyContext context) in /_/src/Orleans.Serialization/Cloning/IDeepCopier.cs:line 366
   at Task<Workspace> OrleansCodeGen.ConfigurationService.Interfaces.Grains.Proxy_IWorkspaceGrain.global::ConfigurationService.Interfaces.Grains.Traits.IEntityGrain<ConfigurationService.Contracts.Workspace>.Initialize(?) in /src/src/ConfigurationService/ConfigurationService.Interfaces.Grains/Orleans.CodeGenerator/Orleans.CodeGenerator.OrleansSerializationSourceGenerator/ConfigurationService.Interfaces.Grains.orleans.g.cs:line 963
   at async Task<ActionResult<Workspace>> ConfigurationService.Api.Controllers.WorkspacesController.Post(CreateWorkspaceRequest request) in /src/src/ConfigurationService/ConfigurationService.Api/Controllers/WorkspacesController.cs:line 42
   at object lambda_method5(Closure, object)
   at async ValueTask<IActionResult> Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor+AwaitableObjectResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, object controller, object[] arguments)
   at async Task Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeActionMethodAsync()+Logged(?)
   at async Task Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeNextActionFilterAsync()+Awaited(?)
   at void Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
   at Task Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
   at Task Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeInnerFilterAsync()
   at async Task Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeFilterPipelineAsync()+Awaited(?)
   at async Task Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeAsync()+Logged(?) x 2
   at async Task Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext)+AwaitRequestTask(?)
   at async Task Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at async Task Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
   at async Task Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at async Task Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at async Task Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

looks a bit related to #8237 .

Also I'm getting the same error for external models like JsonSchema from NJsonSchema library.

After changing the IEnumerable<T> to List<T> and commenting out the JsonSchema property it all works.

In Gigya we have a huge codebase running Orleans (currently pre-v7 version) but it seems that migration to the new v7 will be a bit of a chore if we have to sift out all of these bugs, especially in run time. Any recommendations how to go around it? maybe we should just change the serializer to a STJ or Newtonsoft serializer?

ReubenBond commented 1 year ago

Arbitrary IEnumerable<T> instances cannot be represented on the wire faithfully since the underlying types aren't serializable.

We could have a codec which automatically enumerates your IEnumerable<T> types to a List<T> to avoid this. Is that the behavior you would expect, or would you expect the concrete type identity to maintained across the wire?

EugeneKrapivin commented 1 year ago

I would assume that if the underlaying type is serializable (i.e., List<T>, HashSet<T>, Dictionary<TKey,TValue>) it would be serialized (assuming T has the required GenerateSerializer attribute) by enumerating it, as you suggested. There are some edge cases for IEnumerable<T> like the Enumerable.Empty<T>() which is commonly used to conserve memory instead of allocating an empty List<T> or avoid returning a null where a collection is expected.

As for the underlaying type, I wouldn't expect it to be conserved over the wire, I think it might be a nightmare for model evolution. It is probably possible to guess the collection type from the property type in the model. if the type is IEnumerable<T>, just use a common collection list List<T> since the author doesn't seem to care about the type of the collection (maybe a default collection type setting?)

The new serializer feels a bit stricter than what we use currently (Newtonsoft) in production (Orleans v3.7 IIRC). Is there an analyzer to create warnings on models returned from an IGrainWithXXXKey if they are not serializable at runtime? we have a couple of hundreds of services using Orleans in production, I'm afraid it could be not feasible to use the new serializer safely. Ideas?

SebastianStehle commented 1 year ago

As for the underlaying type, I wouldn't expect it to be conserved over the wire, I think it might be a nightmare for model evolution.

But this could cause performance issues. If you initialize the IEnumerable property with a HashSet and then send it to another grain it could become a list and then contains would be way slower. I think the goal of the new serializer was to be more explicit and the rules should be strict.

EugeneKrapivin commented 1 year ago

@SebastianStehle I see your point. but there must be a sensible solution, it will require significant changes in production code bases. Maybe a flag to allow it, an analyzer to warn. Having these errors at runtime is a big issue (IMHO) for companies with large code-bases based on orleans.

SebastianStehle commented 1 year ago

But is the change to Orleans 7 not a big change anyway?

ReubenBond commented 1 year ago

The only way this ever worked was via fallback serialization, by the way. That is, BinaryFormatter or ILBasedSerializer. It was fragile, and a .NET upgrade could break it.

@SebastianStehle is correct in that we are intentionally aiming for you to explicitly indicate what you want to serialize.

This will only occur when you are using IEnumerable<T> implementations that Orleans doesn't know how to serialize. In this case, that's EmptyPartition<T>.Instance>, an internal .NET type. This kind of thing will happen when you use LINQ (Select, Where, etc) and don't eagerly enumerate the results to a collection at some point. It can also happen when you create IEnumerable methods using C#'s in-built support for that (yield return).

Here are some options:

EDIT: another option is that you can use Newtonsoft.Json for types from your assemblies like you do today. Use the Microsoft.Orleans.Serialization.NewtonsoftJson package for that and see the guide here: https://learn.microsoft.com/en-us/dotnet/orleans/host/configuration-guide/serialization-configuration?pivots=orleans-7-0#configure-orleans-to-use-newtonsoftjson

EugeneKrapivin commented 1 year ago

@SebastianStehle

But is the change to Orleans 7 not a big change anyway?

Yes, it is very big. However, we have hundreds of microservices running orleans, across multiple datacenters and multiple products. I'm currently playing with a pet project and stumbled upon this issue, but migration in our company is on the horizon, and it frightens me a bit.

@ReubenBond I would assume that the serializer would try to enumerate the IEnumerable<T> using a ToList() or ToArray() call in this specific case. This also could be closed behind a serializer configuration toggle so that it doesn't break existing behavior and doesn't incur unexpected performance penalties. The default enumeration method and target deserialization types could also be configured I guess.

Thanks for the pointer to the serialization config page. I'll save it, it looks like a great starting point for us to move since we are already using newtonsoft in production (ouh the fun we had when migrating from .net framework 472 to net5 while serializing the $type)

I would suggest, maybe an analyzer to check up types that will be serialized by orleans and might have problems like missing GenerateSerializer attribute, unknown type (IEnumerable<T> as deserialization target) and stuff like that. I think it might help with the migration process of Orleans 3 users.

Thanks for your responses. I'll be closing the issue since I feel you've answered my questions and provided the information necessary to resolve my issue 😄

ReubenBond commented 1 year ago

@EugeneKrapivin what you're asking for sounds reasonable to me.

I would suggest, maybe an analyzer to check up types that will be serialized by orleans and might have problems like missing GenerateSerializer attribute, unknown type (IEnumerable as deserialization target) and stuff like that. I think it might help with the migration process of Orleans 3 users.

We have something like this, but it cannot practically be done at build/development time in any satisfactory way. This is the same problem AOT has. We cannot tell what the runtime type of a field marked IEnumerable<T> will be and we don't know if you will have registered a custom serializer (eg, JSON). At startup, we check all types using in grain interface method signatures and all types referenced from there (if interested, search the codebase for AnalyzeSerializerAvailability). The configuration flag is TypeManifestOptions.EnableConfigurationAnalysis, which is enabled in the "Development"' environment by default.