Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ERR_HTTP_HEADERS_SENT with 5.3.0 / 5.3.1 and https-proxy-agent configured #17520

Closed
7 tasks done
MelkorCC opened this issue Jun 19, 2024 · 5 comments · Fixed by #17563
Closed
7 tasks done

ERR_HTTP_HEADERS_SENT with 5.3.0 / 5.3.1 and https-proxy-agent configured #17520

MelkorCC opened this issue Jun 19, 2024 · 5 comments · Fixed by #17563
Labels
pending triage regression The issue only appears after a new release

Comments

@MelkorCC
Copy link

MelkorCC commented Jun 19, 2024

Describe the bug

In 5.3.0 7b0a65e added custom logic that utilizes the proxyReq event to set the origin header. This throws an ERR_HTTP_HEADERS_SENT error for me when using using with https-proxy-agent (for example required in setups in which a corporate proxy is in place). I suspect the problem is the same as described in chimurai/http-proxy-middleware#957 and similar / related to the problem described in chimurai/http-proxy-middleware#472 (explanation provided in chimurai/http-proxy-middleware#472 (comment)).

When downgrading to 5.2.13 everything works again.

Reproduction

https://stackblitz.com/edit/github-sadccs?file=src%2FApp.vue,vite.config.js

Steps to reproduce

Run npm install
Replace https://example.org with an actual https proxy in vite.config.js (i don't know a public available one, sorry)
Run npm run dev

First click on the "Without proxy agent" button, you should see the expected result then.
Now click on the "With proxy agent" button, the ERR_HTTP_HEADERS_SENT should appear in the console.

System Info

Binaries:
    Node: 18.15.0
    npm: 9.5.0

Used Package Manager

npm

Logs

Click to expand!
node:_http_outgoing:663
    throw new ERR_HTTP_HEADERS_SENT('set');
    ^

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at new NodeError (node:internal/errors:399:5)
    at ClientRequest.setHeader (node:_http_outgoing:663:11)
    at setOriginHeader (file:///-------/node_modules/vite/dist/node/chunks/dep-BcXSligG.js:61768:16)

Validations

Copy link

stackblitz bot commented Jun 19, 2024

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

@pfdgithub
Copy link

pfdgithub commented Jun 19, 2024

Encountered the same issue.

The exception comes from a commit introduced in vite@5.3.0-beta.0 (7b0a65e).
This commit will modify the origin in the proxied request header by default, sometimes it will throw exception Cannot set headers after they are sent to the client.

In my case, I observed that the origin does not exist in the GET request header, and no exception is triggered. However, the origin exists in the POST request header, which triggers an exception. (why?)
Modify the origin in request headers is only necessary when the server-side explicitly checks for it. In most other scenarios, it is not required.

Three solutions:

  1. Disable modify of origin by default
  2. Check proxyReq.headersSent before modifying origin
  3. Delete origin before entering the vite proxy

@bluwy bluwy added the regression The issue only appears after a new release label Jun 24, 2024
@bluwy
Copy link
Member

bluwy commented Jun 24, 2024

2. Check proxyReq.headersSent before modifying origin

This seems the be the best-fix for me, but I'm not too familiar with the original change.

@johnhunter @sapphi-red do you have thoughts on this?

@johnhunter
Copy link
Contributor

Option 2 sounds like a good solution. I'll have a proper look when I'm back at my desk.

@johnhunter
Copy link
Contributor

I'll address this along with #17562

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pending triage regression The issue only appears after a new release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants