zio / zio-http

A next-generation Scala framework for building scalable, correct, and efficient HTTP clients and servers
https://zio.dev/zio-http
Apache License 2.0
798 stars 402 forks source link

Netty pipeline errors cannot be observed by middleware #2725

Open jgranstrom opened 8 months ago

jgranstrom commented 8 months ago

This means requests rejected by Netty, for example due to request size (default 100kb) will silently fail while responding 413, and no request logging or metric middleware will be able to observe this and likely some other failure modes.

See https://discord.com/channels/629491597070827530/819703129267372113/1217054850966229082

jdegoes commented 5 months ago

This is a problem because middleware only affects requests and responses. So there's no nice way to model pipeline errors.

The ideal solution would be a "middleware tower", where the highest level can handle any handler, the lower level can handle request/response. However, I think that's probably out of scope for ZIO HTTP 3.0

jgranstrom commented 4 months ago

This is a problem because middleware only affects requests and responses. So there's no nice way to model pipeline errors.

The ideal solution would be a "middleware tower", where the highest level can handle any handler, the lower level can handle request/response. However, I think that's probably out of scope for ZIO HTTP 3.0

Sounds good to me! Any plans on adding a level like that after 3.0? It would also be quite helpful for things like capturing lower level errors in middleware I think, which is a separate but somewhat related topic. Such as capturing a stack trace in an error logging middleware.

987Nabil commented 4 months ago

@jdegoes do you mean 3.0 or 3.x?

brodin commented 2 months ago

@jdegoes do you mean 3.0 or 3.x?

Congrats on 3.0 @jdegoes! With it in release do you think this above is likely to land in some iteration of 3.x or is more likely for a eventual 4.0?

jdegoes commented 1 month ago

I think this requires a design.

A simple one would be to add a new field to Routes:

final case class Routes[-Env, +Err](routes: Chunk[zio.http.Route[Env, Err]], fatalError: Throwable => Option[Response])

(Now, it cannot be done in this way, because of backward compatibility--but by inlining the case class, and turning this into an ordinary class, we can make it possible.)

Then the composition semantics would be: the error is passed to both left and right, but the first-non-None response is used ("first-one-wins, but side-effects are possible").

Then Netty server would have to be modified to actually call this function in the event of some pipeline error that prevents the formation of a Request.

Thoughts?