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

Stop using http-proxy? #102

Open
dmp42 opened this issue Apr 4, 2014 · 6 comments
Open

Stop using http-proxy? #102

dmp42 opened this issue Apr 4, 2014 · 6 comments
Assignees
Milestone

Comments

@dmp42
Copy link
Member

dmp42 commented Apr 4, 2014

Between 1.0.2 and 1.0.3:

  • they broke websockets
  • they broke our test_parseerror_body_too_short test (which now returns 200 every time)

The quantity of code involved for http proxying is rather small, and we don't care (for now) about middlewares and other sugar.

So, I will definitely pin it to 1.0.2 for now. But what about we just stop using http-proxy (at least for http)?

Benefits:

  • we control what happens
  • we can get down to it with these leaks
  • we remove the extra stuff that we don't need

Downsides:

  • we are on our own

I'm not sure yet about websockets - but I care chiefly about http right now.

@dmp42 dmp42 added this to the 0.3 milestone Apr 4, 2014
@dmp42 dmp42 added the question label Apr 4, 2014
@samalba
Copy link
Contributor

samalba commented Apr 4, 2014

Good question, why not. We should provide a feature complete replacement if we go for this. I think it manages websocket through an external library.

@dmp42
Copy link
Member Author

dmp42 commented Apr 5, 2014

Short answer is "they don't" - but I'm reluctant to go into possibly breaking changes with websockets without any ws tests. Depending on the outcome of our memory hogging problem, I may push this to 0.4 instead.

@dmp42
Copy link
Member Author

dmp42 commented Apr 14, 2014

Note to self: current version 1.1.1 brings nothing over what we use (1.0.2)

@cywjackson
Copy link

we are running into issue that'd appear has been addressed by 1.0.3: http-party/node-http-proxy#570

Previously I have:

npm list http-proxy -g

/usr/local/lib
└─┬ hipache@0.3.1
└── http-proxy@1.0.2
when installing hipache via "# npm install hipache -g"

But we run into the FD leak issue. This is confirmed by taking out hipache in between and connect to the backend webserver directly.

After finding out http-party/node-http-proxy#570 , I reinstall hipache with modifying the package.json for http-proxy to 1.0.3. I reinstall it via:
"#npm install /mypath/to/hipache -g"

npm list http-proxy -g

/usr/local/lib
├─┬ hipache@0.3.0
│ └── http-proxy@1.0.3
└── http-proxy@1.0.3

And this confirms the FD leak is resolved.

I understand and agree the reasoning of not putting 1.0.3 (since it breaks websockets). But how do you plan to address the FD leak issue? Thx!

@dmp42
Copy link
Member Author

dmp42 commented Apr 29, 2014

Unfortunately the bug is in node core, and not in http-proxy - their "patch" brushes the underlying node problem under the rug, but breaks subtly in a number of ways (that they don't test).

I'll look into this and see if I can find something satisfactory.

@dmp42
Copy link
Member Author

dmp42 commented Apr 29, 2014

@cywjackson we can discuss this at #141 - thanks a lot for your report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants