Bifurcated middleware kind of ugly + potentially fragile #788
Description
Just one I noticed while trying to implement a middleware that does encryption. Here's some lines from the test suite that show roughly what it's like to use this middleware:
middleware := riverencrypt.NewEncryptMiddleware(riversecretbox.NewSecretboxEncryptor(key))
config := &river.Config{
JobInsertMiddleware: []rivertype.JobInsertMiddleware{middleware},
WorkerMiddleware: []rivertype.WorkerMiddleware{middleware},
}
The middleware has to go into both JobInsertMiddleware and WorkerMiddleware. This is pretty unsightly, but also makes things fragile -- if you added the middleware to one and forgot the other, it won't work.
Another related, but separate problem is that while workers can implement work-specific middleware, job args don't have their own variant of that, so something like a worker-specific encryption middleware is currently not possible.
I think there's a chance that maybe what we should have done here was to do something like river.WorkerDefaults like MiddlewareDefaults that lets a middleware struct implement run on insertion, run on work, or both, and which would allow for a more streamlined configuration like:
config := &river.Config{
Middleware: []rivertype.Middleware{middleware},
}
And then maybe have a single Middleware() function on either JobArgs or Worker that would job/worker-specific middleware, so that once again, it wouldn't be possible to accidentally forget middleware from one location or the other.
Category-specific middleware does have some advantages like shallower stack traces in case of a problem (you might otherwise have 2x the middleware in your trace as everything got mixed together), but it kind of feels like what we have here isn't quite right ergonomically.