Bifurcated middleware kind of ugly + potentially fragile

[brandur]

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.

[brandur]

@bgentry I wrote this up a few days ago, but mainly wanted to submit it now so I can reference it from another PR informationally.

[brandur]

Middleware now supports a non-bifurcated version.