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

FiberModule.Key_DoNotSerialize not working #1498

Closed
kbrimble opened this Issue Oct 24, 2016 · 29 comments

Comments

@kbrimble
Contributor

kbrimble commented Oct 24, 2016

Hi,

I have the following code in my Global.asax.cs:

        protected void Application_Start()
        {
            GlobalConfiguration.Configure(WebApiConfig.Register);
            var builder = new ContainerBuilder();
            builder.RegisterType<AgendaSearchService>().Keyed<IAgendaSearchService>(FiberModule.Key_DoNotSerialize).AsImplementedInterfaces().SingleInstance();

            builder.RegisterType<AgendaDialog>().As<IDialog<object>>().InstancePerDependency();

            // Get your HttpConfiguration.
            var config = GlobalConfiguration.Configuration;

            // Register your Web API controllers.
            builder.RegisterApiControllers(Assembly.GetExecutingAssembly());

            // OPTIONAL: Register the Autofac filter provider.
            builder.RegisterWebApiFilterProvider(config);

            // Set the dependency resolver to be Autofac.
            var container = builder.Build();
            config.DependencyResolver = new AutofacWebApiDependencyResolver(container);
        }

And my dialog is constructed like so:

{
    [Serializable]
    [LuisModel("modelId", "subscriptionId")]
    public class AgendaDialog : LuisDialog<object>
    {
        private readonly IAgendaSearchService _searchService;

        public AgendaDialog(IAgendaSearchService searchService)
        {
            SetField.NotNull(out _searchService, nameof(searchService), searchService);
        }

But I still get the Type

'Namespace.AgendaSearchService' in Assembly 'Bot Application1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' is not marked as serializable.

exception when trying to resolve the dependencies for my dialog.

My root dialog is resolved using an auto-factory in my controller:

        private readonly Func<IDialog<object>> _makeRootFactory;

        public MessagesController(Func<IDialog<object>> makeRootFactory)
        {
            _makeRootFactory = makeRootFactory;
        }

Is there something that I am missing?

@willportnoy

This comment has been minimized.

Member

willportnoy commented Oct 24, 2016

It seems likely the singleton service implementing IAgendaSearchService must still be marked serialized. With the "do not serialize" key, we use a serialization surrogate to write out the type [name] so that when deserialized, the service instance can be recovered from the container with that type [name] as a key.

@willportnoy willportnoy self-assigned this Oct 24, 2016

@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Oct 25, 2016

Does this mean that even if the implementation of IAgendaSearchService is not serializable it won't matter if I mark it as such because it won't be serialised anyway?

What about external dependencies that are not marked as serializable? If they need to be tagged anyway, doesn't that defeat the purpose?

@willportnoy

This comment has been minimized.

Member

willportnoy commented Oct 25, 2016

(I'm not completely convinced that the [Serializable] attribute is required to use FiberModule.Key_DoNotSerialize)

Regardless, you can register any serialization surrogates you want in the container - for example, here are some we use:

https://github.com/Microsoft/BotBuilder/blob/master/CSharp/Library/Fibers/Serialization.cs#L51

@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Oct 25, 2016

(I'm not completely convinced that the [Serializable] attribute is required to use FiberModule.Key_DoNotSerialize)

My code didn't work without that attribute, it was the whole point of raising the issue.

I'm confused as why I would want to use the serialization surrogate when I thought that the whole point of the DoNotSerialize key was to avoid having to do this in the first place.

My understanding was that all I had to do was add .Keyed<IAgendaSearchService>(FiberModule.Key_DoNotSerialize) to my binding to allow my dependency to not be serialized but to be injected into the dialog but that doesn't seem the case. That's why I think I must be missing something.

@Maarten88

This comment has been minimized.

Contributor

Maarten88 commented Oct 29, 2016

I have experienced the same problem. I did get a nonserializable service working in my Dialog after a lot of trial and error, my guess is that the Key_DoNotSerialize registration only works if you register and call your Dialog with the MakeRoot methods.

@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Nov 1, 2016

@Maarten88 I am calling my dialog with the make root method but still can't get my non-serializable objects injected.

@tomlm tomlm added bug .NET SDK labels Nov 5, 2016

@grbinho

This comment has been minimized.

grbinho commented Nov 16, 2016

We have the same issue. Every service we tag with FiberModule.Key_DoNotSerialize is still being serialized.

As far as I was able to tell, problem is that Conversation class creates a separate static container that registers only DialogModule that in turn registers FiberModule. FiberModule has the code to enumerate all the services that are marked with that key. In this method.
Because this is all static and private, we are unable to add additional Autofact modules to it.
Maybe there is an option to do something during runtime, but I'm not that familiar with Autofact.

We worked around it by creating our own Coversation class, by basically copying the code and adding additional Autofact modules to the static constructor.

@Maarten88

This comment has been minimized.

Contributor

Maarten88 commented Nov 18, 2016

@grbinho you can do builder.Update(Conversation.Container) to override global registrations in there. It's totally nonobvious how Autofac allows this, took me some time to figure out too.
I use that now to register my (non serializable) services, so that I can resolve them inside Forms.

@grbinho

This comment has been minimized.

grbinho commented Nov 18, 2016

@Maarten88 Tnx for the tip. Much better solution :)

@chrimc62

This comment has been minimized.

Member

chrimc62 commented Nov 27, 2016

@willportnoy can you comment or close appropriately?

@willportnoy

This comment has been minimized.

Member

willportnoy commented Nov 27, 2016

@chrimc62 I think the resolution is that services may need to marked as [Serializable] to use FiberModule.Key_DoNotSerialize. Otherwise, the available options are listed here: https://docs.botframework.com/en-us/technical-faq/#how-can-i-reference-non-serializable-services-from-my-c-dialogs

@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Nov 27, 2016

@willportnoy the issue that I originally raised was that marking a dialog that has non-serializable constructor parameters as serializable throws exceptions on serialisation when using FiberModule.Key_DoNotSerialize. Your solution doesn't solve that problem. I still believe that this is a bug. I can provide more examples if necessary.

@willportnoy

This comment has been minimized.

Member

willportnoy commented Nov 27, 2016

I'm happy to reopen for further discussion. The "do not serialize" key just means "recover the object instance, keyed by object type, from the container on deserialization". As far as I know, it still has to be marked with the serializable attribute. I'd be happy to fix, if it's fixable.

Longer term, we might change the serialization framework as we move to .net core.

@willportnoy willportnoy reopened this Nov 27, 2016

@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Nov 27, 2016

My guess is that it's still trying to serialize the instance rather than ignore it and then recover it later.

I've had a brief look at the code and on serialization it does check if it has the Key_DoNotSerialize but it looks like it's not being respected for whatever reason.

@grbinho

This comment has been minimized.

grbinho commented Nov 28, 2016

@kbrimble

I had the same issue (started with code from Alarmbot sample). As far as I was able to tell from the framework code, "do not serialize" only applies to the Autofac container that was registered in a static Conversation class. Don't know it this is by design, or a bug.

As @Maarten88 suggested, you can do something like this in your Global.asax

// Set the dependency resolver to be Autofac.			
builder.Update(Conversation.Container);
config.DependencyResolver = new AutofacWebApiDependencyResolver(Conversation.Container);
@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Nov 28, 2016

@grbinho we've been creating a new container in addition to the Conversation.Container. I'll see if registering our dependencies into the Conversation container makes a difference.

@carlosscastro

This comment has been minimized.

Member

carlosscastro commented Mar 10, 2017

@kbrimble I noticed this issue is still open. Do you need any further help on this?

@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Mar 13, 2017

@carlosscastro I'll have a look today as to whether using the technique described in this comment solves my issue and close if so 👍

@carlosscastro

This comment has been minimized.

Member

carlosscastro commented Mar 13, 2017

Great @kbrimble , let us know how that goes.

@jsiegmund

This comment has been minimized.

jsiegmund commented Apr 17, 2017

@grbinho @Maarten88 anything else you need to do to get it working? When I replace the normal builder.Build() with builder.Update(), my project loses the ability to resolve the LuisService instance that I'm registering in my module class. Using builder.Build() it resolves fine but I have the same serialization issues.

@grbinho

This comment has been minimized.

grbinho commented Apr 17, 2017

@jsiegmund
Here is how we set it up in our startup class.
In MyAppModule you can set up all other registrations. Or create additional modules. You get the idea.

public void Configuration(IAppBuilder appBuilder)
{
    SetupRegistrations();
    HttpConfiguration.MapHttpAttributeRoutes();   

    HttpConfiguration.Routes.MapHttpRoute(
        name: "DefaultApi",
        routeTemplate: "api/{controller}/{id}",
        defaults: new { id = RouteParameter.Optional }
    );

    appBuilder.UseWebApi(HttpConfiguration);
}

private static void SetupRegistrations()
{
    // http://docs.autofac.org/en/latest/integration/webapi.html#quick-start
    var builder = new ContainerBuilder();
    // In this module we wire up our DI
    builder.RegisterModule(new MyAppModule());

    // Register your Web API controllers.
    builder.RegisterApiControllers(Assembly.GetExecutingAssembly());

    // OPTIONAL: Register the Autofac filter provider.
    builder.RegisterWebApiFilterProvider(HttpConfiguration);

    // Set the dependency resolver to be Autofac.
    builder.Update(Conversation.Container);

    HttpConfiguration.DependencyResolver = new AutofacWebApiDependencyResolver(Conversation.Container);
}
@nwhitmont

This comment has been minimized.

Contributor

nwhitmont commented May 24, 2017

@kbrimble Are you still experiencing the issue after upgrading to BotBuilder 3.8.1?

@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Jun 1, 2017

I've gone through this with the latest version of the BotBuilder and am still having issues. I've built a new project to keep things clean and this is what it looks like.

Service that I want to inject into my dialogs:

public interface IHelloService
{
    string GetMessage();
}

public class HelloService : IHelloService
{
    public string GetMessage() => "hello";
}

AutoFac module with my registrations:

public class HelloModule : Module
{
    protected override void Load(ContainerBuilder builder)
    {
        builder.RegisterType<HelloService>().Keyed<IHelloService>(FiberModule.Key_DoNotSerialize).As<IHelloService>();
    }
}

My Global.asax.cs:

public class WebApiApplication : HttpApplication
{
    protected void Application_Start()
    {

        void UpdateContainer(ContainerBuilder builder)
        {
            builder.RegisterModule<HelloModule>();
            builder.RegisterApiControllers(Assembly.GetExecutingAssembly());
        }

        Conversation.UpdateContainer(UpdateContainer);
        GlobalConfiguration.Configuration.DependencyResolver = new AutofacWebApiDependencyResolver(Conversation.Container);

        GlobalConfiguration.Configure(WebApiConfig.Register);
    }
}

And then a snippet for my controller. I'm injecting a func to use as my root factory that should come from the container:

public class MessagesController : ApiController
{
    private readonly Func<IDialog<object>> _rootFactory;

    public MessagesController(Func<IDialog<object>> rootFactory)
    {
        _rootFactory = rootFactory;
    }

    /// <summary>
    /// POST: api/Messages
    /// Receive a message from a user and reply to it
    /// </summary>
    public async Task<HttpResponseMessage> Post([FromBody]Activity activity)
    {
        if (activity.Type == ActivityTypes.Message)
        {
            await Conversation.SendAsync(activity, _rootFactory);

The error I get when trying to activate the root dialog factory func looks like this:

An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = MessagesController (ReflectionActivator), Services = [BotInjection.MessagesController], Lifetime = Autofac.Core.Lifetime.CurrentScopeLifetime, Sharing = None, Ownership = OwnedByLifetimeScope ---> Unable to resolve the type 'System.Func1[Microsoft.Bot.Builder.Dialogs.IDialog1[System.Object]]' because the lifetime scope it belongs in can't be located. The following services are exposed by this registration:\n- System.Func1[[Microsoft.Bot.Builder.Dialogs.IDialog1[[System.Object, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]], Microsoft.Bot.Builder, Version=3.8.1.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35]]\r\n\nDetails ---> No scope with a tag matching 'Microsoft.Bot.Builder.Dialogs.Internals.DialogModule' is visible from the scope in which the instance was requested.\n\nIf you see this during execution of a web application, it generally indicates that a component registered as per-HTTP request is being requested by a SingleInstance() component (or a similar scenario). Under the web integration always request dependencies from the dependency resolver or the request lifetime scope, never from the container itself. (See inner exception for details.) (See inner exception for details.)

This looks like it is caused by the controllers having a different scope to things registered by the BotBuilder. The type Func<IDialog<object>> is registered using the DialogModule.LifetimeScopeTag named scope and this doesn't exist at the time of activating the controller.

@EricDahlvang

This comment has been minimized.

Collaborator

EricDahlvang commented Jun 1, 2017

This commit: fdb443a addresses the AutoFac FiberModule.Key_DoNotSerialize issue

@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Jun 2, 2017

@EricDahlvang I'll pull down the latest BotBuilder code and see use what's in master to see if it resolves my issue.

@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Jun 2, 2017

I cloned the BotBuilder project and replaced all of my Bot.Builder and Bot.Connector NuGet references with the equivalent project references and I am still seeing the same issue.

@kbrimble

This comment has been minimized.

Contributor

kbrimble commented Jun 5, 2017

I've had another go at this, following the new example in the AlarmBot sample. I'm now falling over on this line in DialogModule_MakeRoot

I've tried both explicitly registering the dialog using builder.RegisterType<RootDialog>().As<IDialog<object>>(); and not doing any registration of IDialog and both produce the same result

Is there anything that I need to do with my registration or do I simply wait for the TODOs to be finished?

@msmohan msmohan closed this Jun 15, 2017

@msmohan msmohan reopened this Jun 15, 2017

@msmohan

This comment has been minimized.

Contributor

msmohan commented Jun 15, 2017

@kbrimble
Can you please share your version of code, I will retry it again.

@msmohan msmohan self-assigned this Jun 15, 2017

@fanidamj

This comment has been minimized.

fanidamj commented Oct 19, 2017

Hoping that issue has resolved and closing it. Reopen if any questions.

@fanidamj fanidamj closed this Oct 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment