vapor / fluent-kit

Swift ORM (queries, models, and relations) for NoSQL and SQL databases
MIT License
211 stars 116 forks source link

Awaiting Middleware operation strange behavior #480

Open gregcotten opened 2 years ago

gregcotten commented 2 years ago

Hi there! Using Vapor 4.54.0 and Fluent 4.4.0

I'm having an issue with AsyncModelMiddleware and perhaps just ModelMiddleware in general.

For simplicity, I've reproduced the issue in the template app that you get from vapor new

Inside Todo.swift:

final class Todo: Model, Content {
    static let schema = "todos"

    @ID(key: .id)
    var id: UUID?

    @Field(key: "title")
    var title: String

    init() { }

    init(id: UUID? = nil, title: String) {
        self.id = id
        self.title = title
    }

    enum MyGreatError: Error {
        case greatError
    }

    func saveAndFail(on db: Database) async throws {
        try await db.transaction { db in
            try await self.save(on: db)
            throw MyGreatError.greatError
        }
    }
}

struct TodoMiddleware: AsyncModelMiddleware {
    func create(model: Todo, on db: Database, next: AnyAsyncModelResponder) async throws {
        let caughtError: Bool

        do {
            try await next.create(model, on: db)
            caughtError = false
        } catch Todo.MyGreatError.greatError {
            db.logger.info("Successfully caught error!")
            caughtError = true
        }

        assert(caughtError)
    }
}

In routes:

func routes(_ app: Application) throws {
    app.get("fail") { req -> String in
        do {
            try await Todo(title: "Fail").saveAndFail(on: req.db)
        } catch {
            return error.localizedDescription
        }

        return "Should never see this"
    }
}

I made sure to add the middleware in the configure step:

app.databases.middleware.use(TodoMiddleware())

Unfortunately my assertion in TodoMiddleware always fails (no error gets thrown). Is there something I am missing, or is this expected behavior?

gregcotten commented 2 years ago

I should mention that the new Todo model correctly doesn't get created on the database, which would be much more frightening!

madsodgaard commented 2 years ago

@gregcotten This happens because the actual try await self.save() does not fail. It is the wrapping transaction that fails, and therefore just causes a ROLLBACK and propagates the error up the chain. The middleware is not aware of any code that happens after your call to .save(). The only errors you would be able to catch are errors from other middlewares or actual errors thrown inside the Fluent create function.

Now whether this behaviour should be changed for transactions, I am not sure about. Let's ask @gwynne

gregcotten commented 2 years ago

@gwynne and @0xTim interested to hear your feedback. Alternatively would be nice to have another way to await once the transaction succeeds.