expressots / adapter-express

Expressjs adapter for ExpressoTS Framework 🐎
https://expresso-ts.com/
MIT License
6 stars 1 forks source link

UploadFile decorator makes return value of original method undefined. #115

Open LhonRafaat opened 2 months ago

LhonRafaat commented 2 months ago

Is there an existing issue for this?

Current behavior

Inside UploadFile decorator Multer is used and required. the descriptor.value is assigned to a function as follows

 descriptor.value = function (...args: Array<any>): any {
      const req = args[0] as Request;
      const res = args[1] as Response;
      // const next = args[2] as NextFunction;

      const multerMiddleware: RequestHandler = getMulterMiddleware(upload, options, method);
      multerMiddleware(req, res, (err: any) => {
        if (err) {
          return res.status(400).json({ error: err.message });
        }
        return originalMethod.apply(this, args);
      });
    };

due to the asynchronous nature of the multerMiddleware the value that is returned is undefined because the descriptor.value is completed before the callback excuted, thus results in undefined return.

Steps to reproduce

@Post("/")
    @FileUpload({ fieldName: "file" })
    execute() {
        return { message: "success" };
    }

this will hang the response and cause a timeout.

Expected behavior

It should allow to send the return of the function to the client without explicitly using res.send or its equivelent to stay consistent with the overall implementation of the framework.

Package version

1.81

Node.js version

20.14.0

In which operating systems have you tested?

Other

No response

LhonRafaat commented 2 months ago

@rsaz Just two notes which kind of relates to this issue and the solution at PR #114, first is that the value of originalMethod will be undefined still because of the same problem mentioned in this issue the asynchronus nature of MulterMiddleware you can log it at the handleFactory method

 const value = await (controller[key] as ControllerHandler)(...args); // whatever the value of the method is 
 // it becomes undefined if the FileUpload decorator is used.
console.log(value);

So I dont know if this is gonna cause issues in the future.

Secondly, if a user returns undefined the request hangs and never sends back a response, because of this != "undefined" check here

if (
          res &&
          result &&
          typeof result != "undefined" &&
          !isRequest(result) &&
          !isResponse(result)
        ) {
          return res.send(result);
        }

usually when undefined is returned a response with no body is returned to client, or transformed into a 204 no content response.

These are 2 points to consider so to make sure unexpected behaviors does not happen.