Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fromRequestHandler doesn't handle errors #39

Closed
DenisFrezzato opened this issue May 11, 2021 · 3 comments
Closed

fromRequestHandler doesn't handle errors #39

DenisFrezzato opened this issue May 11, 2021 · 3 comments

Comments

@DenisFrezzato
Copy link
Owner

DenisFrezzato commented May 11, 2021

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

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

@mlegenhausen
Copy link
Contributor

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

DenisFrezzato commented Jun 27, 2021

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.

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

No branches or pull requests

2 participants