sukovanej / effect-http

Declarative HTTP API library for effect-ts
https://sukovanej.github.io/effect-http
MIT License
247 stars 20 forks source link

Route and Group Middlewares #640

Open emrosenf opened 1 month ago

emrosenf commented 1 month ago

@effect/platform allows middleware at the request, router and server level. It would be really neat to do the same here with ApiGroups or RouterBuilder.handle for an individual route

sukovanej commented 1 month ago

Hey, a middleware in /platform is simply a function App -> App (where App is an Effect of a concrete shape). So if a middleware doesn't modify the success channel, you can already apply it for an individual endpoint handler using a composition. It's true there is currently no api to use a middleware for an individual router builder. Do you have any specific use-case in mind?

emrosenf commented 1 month ago

Only RouterBuilder.Build produces an HttpApp.Default that can accept an HttpMiddleware.

setSecurity, setRequestBody, setRequestQuery, etc, are effectively middlewares, but are opinionated in what they can do where they can be applied (ApiGroup and Api)

What if I wanted to set a header on an ApiGroup? Or apply a rate-limit to a few routes? An HttpMiddleware would be a nice way to do this.

You say that it is possible already to apply it to an individual endpoint handler, can you provide a quick example on how to do this? Can the same be done with an ApiGroup?

emrosenf commented 1 month ago

I think this is what you meant by composition?


const effectHttpApp = pipe(
  RouterBuilder.make(api),
  RouterBuilder.handle(getUserHandler),
  RouterBuilder.build,
  withMiddleware("middleware1"),
)

const router = HttpRouter.empty.pipe(
  HttpRouter.mount("/r1", router1),
  // Apply Middleware affecting all routes under /r1
  HttpRouter.use(withMiddleware("M3")),
  // Apply Middleware affecting all routes
  HttpRouter.use(withMiddleware("M4")),
  HttpRouter.mountApp('/r2', effectHttpApp)
)

router.pipe(NodeServer.listen({ port: 3000 }), NodeRuntime.runMain)
sukovanej commented 1 month ago

yeah, that's more or less what I meant, tho I didn't realised the HttpMiddleware is defined using HttpApp.Default (<E, R>(self: App.Default<E, R>) => App.Default<any, any>) so unfortunately middlewares defined using HttpMiddleware.make can't be used per endpoint because application of a middleware will change the success channel to HttpServerResponse, if it was defined as a mapping of HttpApp.HttpApp<A, E, R> it would be possible, e.g.

import { HttpServerRequest } from "@effect/platform"
import { NodeRuntime } from "@effect/platform-node"
import { Schema } from "@effect/schema"
import { Effect, pipe } from "effect"
import { Api, Handler, RouterBuilder } from "effect-http"
import { NodeServer } from "effect-http-node"

const logUrlMiddleware = <A, E, R>(self: Effect.Effect<A, E, R>) =>
  Effect.gen(function*(_) {
    const request = yield* HttpServerRequest.HttpServerRequest
    console.log(request.url)
    return yield* self
  })

const testEndpoint = Api.get("test", "/test").pipe(
  Api.setResponseBody(Schema.String)
)

const testHandler = Handler.make(
  testEndpoint,
  () => Effect.succeed("test").pipe(logUrlMiddleware)
)

const api = Api.make().pipe(
  Api.addEndpoint(testEndpoint)
)

const app = pipe(
  RouterBuilder.make(api),
  RouterBuilder.handle(testHandler),
  RouterBuilder.build
)

app.pipe(NodeServer.listen({ port: 3000 }), NodeRuntime.runMain)
sukovanej commented 1 month ago

setSecurity, setRequestBody, setRequestQuery, etc, are effectively middlewares, but are opinionated in what they can do where they can be applied (ApiGroup and Api)

If we're talking about /platform HttpMiddlewares, then they are not. Api, ApiGroup, ApiEndpoint etc are strictly descriptive objects, they (mostly) encode only structure, no behaviour. You use these to implement server app, or to derive an http client, or an example server, etc. HttpMiddleware, on the other side, encodes only behaviour and no structure, so talking about middlewares for Api* objects doesn't really make much sense. The practical reason to not put behaviour onto the API declarations is that these structures can be used to generate an api client which can be used in a browser so it would pollute the frontend build with a backend code and potentially break things.

What if I wanted to set a header on an ApiGroup? Or apply a rate-limit to a few routes? An HttpMiddleware would be a nice way to do this.

Currently, there is no specific API for it. Tho, you can always do something like this.

const rateLimit = (paths: ReadonlyArray<string>) =>
  HttpMiddleware.make((app) =>
    Effect.gen(function*(_) {
      const request = yield* HttpServerRequest.HttpServerRequest

      if (paths.some((path) => request.url.startsWith(path))) {
        // do your stuff
      }

      return yield* app
    })
  )

const app = pipe(
  RouterBuilder.make(api),
  RouterBuilder.handle(...),
  RouterBuilder.build,
  rateLimit(["/test", "/another"])
)
sukovanej commented 1 month ago

Based on the way I personally use this lib right now, I'm thinking about refactoring (or maybe getting rid of completely) the RouterBuilder and get closer to the /rpc design. Overall, I think the API of this lib should become more compositional, have more granular combinators built probably around Handler data structure so that users can compose routers their way.

So I'm on the same page with you and the limitation you describe is result of the current design which tries too much to hide the underlaying /platform http details while it should rather try to build around it.

sukovanej commented 1 month ago

Oh actually, I forgot there is RouterBuilder.mapRouter. Therefore, even in the current state, you can apply middlewares using this mapping and if you create multiple router builders, you can use middlewares more granularly. Router builders can be merged into a final one using RouterBuilder.merge.

adrian-gierakowski commented 1 month ago

So I guess setSecurity at ApiGroup level is currently not possible?

adrian-gierakowski commented 1 month ago

Based on the way I personally use this lib right now, I'm thinking about refactoring (or maybe getting rid of completely) the RouterBuilder and get closer to the /rpc design.

btw. maybe a bit off topic, but was I was splitting up my endpoints into groups, and was surprised that I had to use the full api spec to create partial RouterBuilders if there are meant to be merged. Initially tried to create partial api specs (one for each group) and then use them when creating partial RouterBuilders, but when merging the builders I got this error at runtime (it compiled correctly)

The problem is that when testing individual groups, I had to implement the entire api, so instead of:

NodeTesting.make(
  RouterBuilder.build(groupRouterbuilder),
  groupApiSpec,
)

I had to do:

NodeTesting.make(
  RouterBuilder.build(ExampleServer.handleRemaining(groupRouterbuilder)),
  groupApiSpec,
)
sukovanej commented 4 weeks ago

The RouterBuilder.build is enforcing all the endpoints are implemented on the type-level using a generic type parameter of the RouterBuilder describing remaining api endpoints. Therefore, the type information about endpoints is not describing implemented endpoints within the builder but instead which endpoints are NOT implemented yet. That's the reason why RouterBuilder always needs to know about the full API.

adrian-gierakowski commented 4 weeks ago

My point is that at type level it all worked with partial apis provided to partial builders but I got an error at runtime when running the merged router

sukovanej commented 4 weeks ago

Ah, my bad, now I got it. For the router builder, it's not possible to recognise different Api instances on the type level so there is at least this runtime check in case of a miss-use.

adrian-gierakowski commented 4 weeks ago

It would be great if one could create a partial router for given partial api, and then compose the partial apis and the partial routers. Currently the partial router seems like just a way of splitting implementation of the final larger api into multiple files, but the partial router on its own is not usable.