DenisFrezzato / hyper-ts

Type safe middleware architecture for HTTP servers
https://denisfrezzato.github.io/hyper-ts/
MIT License
391 stars 18 forks source link

`fromRequestHandler` doesn't handle errors #39

Closed DenisFrezzato closed 3 years ago

DenisFrezzato commented 3 years ago

🚀 Feature request

Current Behavior

There is no way to handle an error returned by a middleware wrapped with fromRequestHandler, the error is just ignored in its implementation.

Desired Behavior

I'd like to have a way to handle a possible error returned by a middleware.

Suggested Solution

In my codebase I have this helper:

import { Request, RequestHandler } from 'express'
import * as E from 'fp-ts/Either'
import { pipe } from 'fp-ts/function'
import * as H from 'hyper-ts'
import { ExpressConnection } from 'hyper-ts/lib/express'

export const fromRequestHandlerEither = <
  I = H.StatusOpen,
  E = never,
  A = never
>(
  requestHandler: RequestHandler,
  onError: (reason: unknown) => E,
  f: (req: Request) => E.Either<E, A>,
): H.Middleware<I, I, E, A> => (c) => () =>
  new Promise((resolve) => {
    const { req, res } = c as ExpressConnection<I>
    requestHandler(req, res, (err: unknown) =>
      err
        ? resolve(E.left(onError(err)))
        : pipe(
            f(req),
            E.map((a): [A, H.Connection<I>] => [a, c]),
            resolve,
          ),
    )
  })

Who does this impact? Who is this for?

People using Express middlewares developed by the community without hyper-ts in mind.

Describe alternatives you've considered

Keep using the aforementioned function.

Your environment

Software Version(s)
fp-ts 2.10.5
hyper-ts 0.6.1
TypeScript 4.2.4
DenisFrezzato commented 3 years ago

We could also deprecate the current fromRequestHandler and remove it in a next release, in favour of fromRequestHandler2V, or something like this.

mlegenhausen commented 3 years ago

Maybe we can overload the function and check if it is used with two or three parameters? I am not a big fan of V2 functions 😅

DenisFrezzato commented 3 years ago

Maybe we can overload the function and check if it is used with two or three parameters?

This is a good idea, I haven't thought of overloads, thanks for the suggestion!. I've created a PR, if you want to check it out.