AddModulesAsync requires all module dependencies to be available

[Nihlus]

Between 1.0.2 and 2.0.0, PR #934 changed the behaviour of AddModulesAsync and ModuleBase when it implemented an overridable callback for modules when they were instantiated and built. In particular, it's this section of ModuleBuilder, where an instance of the module is created to then call the OnModuleBuilding method.

https://github.com/discord-net/Discord.Net/blob/5dad0fa1a1f40c104cec86dde6d1c10a3025d915/src/Discord.Net.Commands/Builders/ModuleBuilder.cs#L126

This has broken most help and documentation generators, since any call to AddModulesAsync and its variants now requires all of the module's dependencies to be available, even in a context where all the developer really wants is a ModuleInfo to extract information from. This makes it extremely problematic to design automated tests which operate on ModuleInfo instances, implement generalized help commands, and, in my case, static documentation site generators. I've personally hit all three of the aforementioned problems, but the last one is where things get really hairy.

In particular, I have an EF Core context which is a dependency of most modules - that is plainly not available in the docsite generator, and really shouldn't be either.

Any known workarounds are welcome. If the behaviour should remain as-is, it'd be nice to see the introduction of a reflection-only method that spits out a ModuleInfo which you can operate on.

[QuantumToasted]

In particular, I have an EF Core context which is a dependency of most modules - that is plainly not available in the docsite generator, and really shouldn't be either.

Without a proper analysis of your code I can only supply what I've done for this. I simply made my EF Core context lazily loaded, so to speak. You can view how I did it for my bot here, but the tl;dr was that in my command context (or if you wish, your ModuleBase), you simply have something akin to:

private MyDatabaseContext _dbContext;

public MyDatabaseContext DbContext => _dbContext ?? (_dbContext = new MyDatabaseContext());

public void Dispose()
{
    _dbContext?.Dispose();
}

I know the command framework I am using will call Dispose() on your ModuleBase if it implements IDisposable, (but I have not used Discord.Net.Commands in a while), and as such you can follow the rabbit hole down to disposing your database context if you needed it for the module.

Of course, if you do not want to do it this way, or I completely misunderstood what I just quoted from you, then my apologies.

[FiniteReality]

I've had a look around and I can't see any easy work-arounds for this. Nor does it seem to be a particularly easy problem to solve.

Right now, I can only think of this "work-around":

serviceCollection.AddSingleton<CommandService>((provider) =>
{
    var service = new CommandService();
    service.AddModulesAsync(Assembly.GetEntryAssembly(), provider)
        .GetAwaiter().GetResult();
    return service;
});

Everything else would throw an exception somewhere in our codebase.

[Joe4evr]

It sounds like you might've been doing things wrong.

• Generalized help commands need only the CommandService instance to be injected, doing a lookup at the time the command is invoked
• Any proper static documentation generator doesn't need runtime instantiation of modules
• For automated tests you simply setup your dependencies with mocks as part of your Arrangement step

[Nihlus]

@Joe4evr That's precisely what I'm saying. My static doc generator, which in the past did not need runtime instantiation of the modules, now transitively requires it because Discord.NET performs an instantiation when adding modules to the command service. I have no way to get a ModuleInfo without adding them to the command service, as far as I've been able to tell.

The help command is mostly fine, yes, but to even get to a working command service, you need to have all dependencies available, which includes the command service for the help module, which requires all dependencies, which requires the command service... you get the point. There's now a cyclical dependency I have no control over. I can work around it with the service provider's lazy loading, sure, but it's a workaround for something that really shouldn't do runtime instantiation in the first place.

[Joe4evr]

My static doc generator, which in the past did not need runtime instantiation of the modules, now transitively requires it because Discord.NET performs an instantiation when adding modules to the command service.

Why would it do that in the first place? If it requires runtime instantiation of all your types, it is by definition not a static generator.

to even get to a working command service, you need to have all dependencies available, which includes the command service for the help module, which requires all dependencies, which requires the command service... you get the point. There's now a cyclical dependency

Not if you setup the entire container prior to registering modules, and by the way, CommandService is special-cased in that it doesn't need to be in the container at all to get injected.

[FiniteReality]

@Joe4evr Their point is that previously, runtime instantiation was not required because Discord.Net only used reflection on the types passed in. Now, implicitly, it does, because we instantiate the types at runtime.

As for setting up the container prior to registering modules, there are people (myself included) who prefer to add their command service to the service provider. With the way this is set up right now, it's almost impossible unless you create a specialized factory for the type a la the code snippet I gave.

[Joe4evr]

As for setting up the container prior to registering modules, there are people (myself included) who prefer to add their command service to the service provider.

Even then, there is absolutely no problem.

private readonly CommandService _commands;
private readonly IServiceProvider _services;
public Program()
{
    _commands = new CommandService();
    _services = ConfigureServices();
}
private IServiceProvider ConfigureServices()
{
    var services = new ServiceCollection()
        .AddSingleton(_commands)
        //......
        ;

    return services.BuildServiceProvider();
}
public async Task StartAsync()
{
    await _commands.AddModulesAsync(/*...*/, _services);
    //......    
}

[Nihlus]

Alright, so there are workarounds in place for the common cases, but that still doesn't leave me with any option for reflection-only inspection of command modules without writing my own reflection code to pull out information about commands.

My static generator is static, and has always been static. I do not instatiate instances of modues; Discord.NET does when creating ModuleInfo instances. This breaks my generator, because that requires all dependencies to be present.

[Joe4evr]

reflection-only inspection of command modules My static generator is static, and has always been static.

This is a contradiction. An actual static tool looks at the raw code as just text and parses the bits it wants out of that. DocFX is a static doc generator, Doxygen is a static doc generator.

The moment you start to use reflection, it is no longer static because reflection is by definition dynamically-bound.

[Nihlus]

We're not talking about the same thing, then; I'm referring to a static site generator, not a static code inspector. The generator uses reflection to generate a static markdown site. Discord.NET's ModuleInfo contains the data I need in a very nice, easily accessible format, and it's worked great in the past.

The issue isn't in how I grab that data or how I use it - it's in that D.NET instantiates a full instance of the module during what is ostensibly a plain introspection stage, preventing plain introspection without having all dependencies on hand. I'd be happy with that if there was an inspection-only alternative beyond writing my own complete ModuleInfo builder.

[Joe4evr]

The way I see it, your issue is thinking that you need introspection in the first place, when you can simply run docfx over your project and get all the markdown you want. This is exactly what we do for the docs of D.Net itself.

[Nihlus]

The documentation generator is specific to D.Net command modules and the commands contained within - it's not for code documentation, it's for command documentation.

Using ModuleInfo and reflection makes that much, much easier to write for and reason about. I don't want or need a full docfx run for this, since I'm not documenting code API.

[foxbot]

In your case this doesn't really seem like a breaking issue. Instead of building your bot's documentation at startup or module initialization, build it after modules have been initialized.

Your DI container should be immutable; meaning that after you build it the first time, you shouldn't touch it again. If you're adding things to your DI container after building modules, your design is the issue.

In the case of "static" documentation, try adding a DocumentationService to your container at startup, initializing the CommandService and adding modules, and then informing your DocumentationService to crawl modules and build a help index.

[Nihlus]

@foxbot The doc generator is in a separate project and is not part of the bot. It references it and inspects it, yes, but it's not a part of the program. I'm not touching the DI container.

[foxbot]

You shouldn't need a full CommandService then, pure reflection should suffice.

[Nihlus]

That loops me right back to asking for alternative API that produces a ModuleInfo. Previously, CommandService filled that niche by providing a unified, reflection-only API when used to simply inspect and load modules. I'd rather not rewrite what amounts to a clone of the already existing module builder when all that's blocking me is D.Net's instantiation of a full module instance during its own reflection stage.

[FiniteReality]

There's no need for an alternative API or anything. There is a bug here which can be fixed. I'm going to experiment with moving the OnBuilding stuff into ModuleClassBuilder, and only calling it if it is overridden.

[csmir]

@Cenngo with lack of followup from previous contributors, do you think this should be followed up on? I personally do not see a reason considering the niche circumstance

[Nihlus]

I've long since moved on from using Discord.NET at all, so I no longer have a dog in the race.