Skip to content
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

Awaiting Middleware operation strange behavior #480

Open
gregcotten opened this issue Jan 8, 2022 · 3 comments
Open

Awaiting Middleware operation strange behavior #480

gregcotten opened this issue Jan 8, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@gregcotten
Copy link

gregcotten commented Jan 8, 2022

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 gregcotten added the bug Something isn't working label Jan 8, 2022
@gregcotten
Copy link
Author

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

@madsodgaard
Copy link
Contributor

@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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants