moleculerjs / moleculer-web

:earth_africa: Official API Gateway service for Moleculer framework
http://moleculer.services/docs/moleculer-web.html
MIT License
292 stars 118 forks source link

ERR_HTTP_HEADERS_SENT when upload file endpoint alias throws error. #200

Open giovanni-bertoncelli opened 3 years ago

giovanni-bertoncelli commented 3 years ago

Issue

I'm facing an issue with moleculer-web file-upload alias... I get a ERR_HTTP_HEADERS_SENT error in the onAfterCall hook when trying to change a res header after the alias has encountered an error and responded...

Possible causes

I think this is an issue with the Busboy implementation... When I throw an error inside an endpoint like this, this catch block (https://github.com/moleculerjs/moleculer-web/blob/f4669daaede6ec55e29e32bd6f32c957e58614e9/src/alias.js#L169) gets executed and then the busboy.on('error'...) sends the request to the client.. The issue is even after the error the busboy.on('finish'...) gets called and then here the ERR_HTTP_HEADERS_SENT error comes since the response has been already sent...

icebob commented 3 years ago

Please provide a repro example.

giovanni-bertoncelli commented 3 years ago

@icebob Yep! here you go: https://codesandbox.io/s/moleculer-project-forked-032zn You can try that by calling POST https://CODE_SANDBOX_BASE_URL/api/greeters/throwerror with a file uploaded.

giovanni-bertoncelli commented 3 years ago

@icebob any update on this?

giovanni-bertoncelli commented 3 years ago

@icebob Sorry to bother... had you have the chance to see this?

icebob commented 3 years ago

I don't see the issue in codesandbox example

image

Fabrest29 commented 2 years ago

I have the same issue.

My workaround :

onAfterCall(
              this: Service,
              ctx: Context,
              route: Record<string, unknown>,
              req: IncomingMessage,
              res: ServerResponse,
              data: Record<string, string>
            ) {
              if (!res.finished) {
                res.setHeader("X-XSS-Protection", "1; mode=block");
              }
              return data;
            },
Licas commented 2 years ago

I have the same issue.

  • in onError : I call the res.end to send a message.

    • when message is sent to client, the response (res: ServerReponse) is finished.
  • in onAfterCall : I setHeader

    • res.setHeader("X-XSS-Protection", "1; mode=block");

=> I get a ERR_HTTP_HEADERS_SENT error in the onAfterCall hook when trying to change a res header after the alias has encountered an error and responded...

My workaround :

onAfterCall(
              this: Service,
              ctx: Context,
              route: Record<string, unknown>,
              req: IncomingMessage,
              res: ServerResponse,
              data: Record<string, string>
            ) {
              if (!res.finished) {
                res.setHeader("X-XSS-Protection", "1; mode=block");
              }
              return data;
            },

This worked for me too, thank you! The only difference is that I used res.writableEnded instead of res.finished

arthurgermano commented 9 months ago

Hi there, same thing here, a lot of issues with busboy and uploading files. If the service is in a node, not the gateway, a lot of issues happens, this part is not well tested. I tried more than one file and get several requests, I tried one file, I can't validate the fields and stop the busboy uploading. Very different from fastify or express where I have total control of the upload.

Anyway moleculer is great, this is the first big issue that I found, I have been working with moleculer for a couple of years now. Excelent project, we can't find any replacement for it, the way it works is outstanding =)

icebob commented 9 months ago

The integrated busboy is just an option for file uploading. You can disable it and add another Express middleware solution like formidable. I've just created another file upload example with formidable:

https://github.com/moleculerjs/moleculer-web/blob/master/examples/formidable/index.js

arthurgermano commented 9 months ago

Hi @icebob .. perfect.. But this is hardly the case. Abusing of your good will, what happens is in a node.. The example shows directly in the apigateway.

Here you have access t么 the request. But what is the case when we are in a node defined by an alias route and using rest notation. More specific the handle function. The handle function does not have access t么 the request but only the context right? And since I am in a big project with several nodes we are using before and after triggers functions t么 validate the paramos before continue with the upload meaning that If the params sent are invalido we discard the request with an error.

Thanks in advanced for your time and patience t么 this matter. 馃檹馃檹

icebob commented 9 months ago

Yes, you have no access to req in remote nodes. Moleculer is a microservices framework and not a web framework, you can't have access to the original request because the request can be transferred to a remote node and the Request instances are not serializable. So you should do req things on the API gateway locally. This is why we process the file uploading in the API GW and send the file as a Stream.

arthurgermano commented 9 months ago

@icebob perfect. Many thanks for the response. I will try to find a way conciliate allowing remote nodes to process the request of the params and the Gateway t么 absorb the upload of the file.

Excelent project 馃檹馃檹