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

Route and Group Middlewares #640

Open
emrosenf opened this issue Jul 26, 2024 · 13 comments
Open

Route and Group Middlewares #640

emrosenf opened this issue Jul 26, 2024 · 13 comments

Comments

@emrosenf
Copy link

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

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

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

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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

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

@adrian-gierakowski
Copy link

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
Copy link
Owner

sukovanej commented Aug 9, 2024

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

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
Copy link
Owner

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants