express-async-errors icon indicating copy to clipboard operation
express-async-errors copied to clipboard

Lint error with TypeScript

Open royalpinto opened this issue 4 years ago • 8 comments

Using the async handler with this lib leads to the following lint error as the return type of async handler is a Promise<> and the expected one is just a void.

Error Message:

Promise returned in function argument where a void return was expected. (@typescript-eslint/no-misused-promises)

I am not sure if the type definition of the express handler can be changed in this lib's definitions. Create a ticket just in case if it's possible.

royalpinto avatar Jun 01 '21 09:06 royalpinto

Me too getting the same error. So I tried doing something like this: export const tempName = (req: Request, res: Response, next: NextFunction) => async () => { // everything else with await }

And then the linting error is gone.

Bikash9609 avatar Sep 13 '21 18:09 Bikash9609

Any updates here?

AxelGreen avatar Feb 23 '23 15:02 AxelGreen

Any updates here?

Can you share some code where you are getting this lint error? I think I know the solution

Bikash9609 avatar Feb 23 '23 15:02 Bikash9609

Error message

ESLint: Promise returned in function argument where a void return was expected.(@typescript-eslint/no-misused-promises)

Code:

import express, { Request, Response } from 'express';
import 'express-async-errors';

async function asyncThrowError(): Promise<void> {
    await Promise.all([]);
    throw new Error('Test error');
}

async function main(): Promise<void> {
    // TODO: remove later
    await Promise.all([]);

    const app = express();

    app.get('/async-error', async (_request: Request, response: Response) => {
        await asyncThrowError();
        response.status(200).json({ success: true });
    });

    app.listen(40512);
}

main()
    .then(() => {
        // eslint-disable-next-line no-console
        console.log('Application started');
    })
    .catch((error: Error) => {
        // eslint-disable-next-line no-console
        console.error('Application startup error', { error });
    });

CleanShot 2023-02-23 at 16 00 01@2x

AxelGreen avatar Feb 23 '23 16:02 AxelGreen

app.get('/async-error', async (_request: Request, response: Response): Promise<void> // Added this => {
    await asyncThrowError();
    response.status(200).json({ success: true });
});

or

app.get('/async-error', async (_request: Request, response: Response) => {
    await asyncThrowError();
    return response.status(200).json({ success: true }); // Added return
});

Let me know if one of them works.

Bikash9609 avatar Feb 24 '23 06:02 Bikash9609

Thank you for your suggestions!

Unfortunately, both versions give me the same issue as before.

CleanShot 2023-02-24 at 11 48 36@2x

AxelGreen avatar Feb 24 '23 11:02 AxelGreen

Same error . Plus , it does not tend to handle the async errors resulting from using Multer for file uploads

code0monkey1 avatar Aug 05 '23 13:08 code0monkey1

~I have the solution!~

EDIT: Unfortunately, fixing the RequestHandler type is only part of the solution:

  • we cannot fix the ErrorRequestHandler type because it's declared as type, not as interface. I assume, we can discuss this change with the maintainer of the types package
  • the express application Application type and the router IRouter type are also affected by the fix, as they are extended from the RequestHandler type. So they will trigger the same eslint error when using them e.g. as an argument for https.createServer

At the moment, this is above the time budget I have for the task. Maybe someone else would like to pick it up from here

The partial solution

  • It will not remove the eslint error from error-handling routes
  • It may introduce a few new instances of this eslint error. Yet, probably much less than will eliminate

Create or extend an existing .d.ts file in your project with the following code (e.g. types-override.d.ts):

import type { Request, Response } from 'express';
import type { ParamsDictionary, Query } from 'express-serve-static-core';
import type { NextFunction } from 'express-serve-static-core';

declare module 'express-serve-static-core' {
  export interface RequestHandler<
    P = ParamsDictionary,
    ResBody = any,
    ReqBody = any,
    ReqQuery = Query,
    LocalsObj extends Record<string, any> = Record<string, any>
  > {
    (
      req: Request<P, ResBody, ReqBody, ReqQuery, LocalsObj>,
      res: Response<ResBody, LocalsObj>,
      next: NextFunction
    ): void | Promise<void>;
  }
}
  • In case you are creating a new file and your tsconfig.json has the "include" section then ensure that the file is covered by the patterns of the section
  • For the TypeScript versions older than 3.8 you can replace import type with just import

You are done!

The type is fixed, so you will see no @typescript-eslint/no-misused-promises) error for your routes

Comments on the implementation

The conflict we are having here is with the type RequestHandler of express, which is extended from the type RequestHandler of the express-serve-static-core - its return type is void, and we are providing a function with the return type Promise<void>. If we patch the RequestHandler of the express it would become incompatible with the root type, this is why we are extending directly the root type

The feature we are using here is called merging interfaces it allows extending existing interfaces by re-declaring them

So, here we duplicate the definition of the RequestHandler type from the express-serve-static-core but change the return type from void to void | Promise<void>

Alexsey avatar May 24 '24 03:05 Alexsey