m-barthelemy / vapor-queues-fluent-driver

A Fluent implementation for https://github.com/vapor/queues (Vapor4)
MIT License
32 stars 16 forks source link

Bug inside ordering of objects in configure() #6

Closed thuotdwz closed 4 years ago

thuotdwz commented 4 years ago

Hi, I have noticed another bug. If inside configure the order of initialization is done so queues driver is before database, like this:

app.queues.use(.fluent(.sqlite))
app.databases.use(.sqlite(.memory))

Application will then receive an error when running migrations:

ERROR: Cannot schedule tasks on an EventLoop that has already shut down. This will be upgraded to a forced crash in future SwiftNIO versions.
ERROR: Cannot schedule tasks on an EventLoop that has already shut down. This will be upgraded to a forced crash in future SwiftNIO versions.

If order is changed into:

app.databases.use(.sqlite(.memory))
app.queues.use(.fluent(.sqlite))

Then the application will run successfully.

I see there is a note in the code

    // How do we report that something goes wrong here? Since makeQueue cannot throw.
    let dialect = (db as? SQLDatabase)?.dialect.name ?? "unknown"
    let dbType = QueuesFluentDbType(rawValue: dialect) ?? .none

Maybe here we can log a warning message using context.logger to let the developer know the initialization order is wrong so they can fix it?

This would be helpful to much more easily tack down the issue. It took me a while to figure out!

Thanks for working on this package, it is useful!

m-barthelemy commented 4 years ago

@thuotdwz I'll look into it, but first of all please do note that for now this driver won't work with Sqlite (as stated in the README). It's Postgres, Mysql or MariaDB only.

m-barthelemy commented 4 years ago

Unfortunately I was unable to reproduce. If I call app.queues.use(.fluent(.sqlite)) before app.databases.use(.sqlite(.memory)), I get Fatal error: No datatabase configuration registered for DatabaseID(string: "sqlite").: file which is pretty explicit despite the typo :-)

thuotdwz commented 4 years ago

Thanks for your reply, could be there is something else in my app causing the shut down event loop issue. Appreciate the readme note!

Also I think SQLITE can be made to work with a small change, I will file a new ticket for thaat.

thuotdwz commented 4 years ago

I found out the problem with ERROR: Cannot schedule tasks on an EventLoop that has already shut down. This will be upgraded to a forced crash in future SwiftNIO versions.

It turns out, we cannot shutdown eventLoopGroup in FluentQueuesDriver shutdown, because this object is not owned by queues driver.