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

Hoxy is preventing Node from exiting #88

Open
sholladay opened this issue Nov 8, 2016 · 2 comments
Open

Hoxy is preventing Node from exiting #88

sholladay opened this issue Nov 8, 2016 · 2 comments

Comments

@sholladay
Copy link
Contributor

sholladay commented Nov 8, 2016

If I so much as require('hoxy'), Node will never exit.

This is problematic for the end-user. They go to Control-C on my server and it never dies, as I trap SIGINT and expect proxy.close() to exit gracefully.

hoxy 3.2.0
node 7.0.0
npm 3.10.9

@sholladay
Copy link
Contributor Author

sholladay commented Nov 15, 2016

I used the why-is-node-running module to debug this and it pointed a finger at the server.listen() in cycle.js.

I'm not very familiar with this portion of the codebase, but the solutions that seem obvious to me are:

  • Only start listening when the user calls listen() on a Hoxy instance. Otherwise, it is surprising that the program remains open forever.
  • Stop listening / close() at a reasonable time so that graceful shutdown works as expected. Options might be:
    • Handle SIGINT and close.
    • Keep track of whether there are listening hoxy instances. Close when there are no more.
    • Use a class instead of a singleton. Listen / close in tandem with each hoxy instance.

sholladay added a commit to sitecues/sitecues-proxy that referenced this issue Nov 21, 2016
Hoxy has a server buried in its internals that it uses to accomplish some neat tricks for serving static files. Unfortunately, there’s no public way to shutdown this server. That conflicts with what we want to do to shutdown gracefully upon receiving SIGINT.

For now, we’re just going to force quit after a reasonable period.

Details: greim/hoxy#88
sholladay added a commit to sitecues/sitecues-proxy that referenced this issue May 30, 2017
* Hapi experiment.

No sitecues injection yet. But most of the infrastructure is there.

* Use zlib streams to handle encoded responses.

This sacrifices CPU for improved network performance, as with most encoding processes. Would be nice to benchmark this at some point.

* Resolve domain root URLs, passthrough non-HTML.

* Deal with edge cases for relative targets.

* Defend against invalid targets.

* Remove obsolete imports.

* Simplify attribute processing and logging.

* Simplify logging.

* Use ES2016 Array#includes to improve readability.

* Sync up target and stream-target routes.

* Breaking up request and response handlers.

* Spacing tweaks.

* Core modules come first.

* Put DOM modification in its own module.

* Remove localStatePassthrough, will never use it.

This config option allows cookies intended for the proxy to be passed upstream. For our use case, this would be a security problem. Don't use it.

* Support XHTML.

* SImplify route import and response header mapping.

* Standardize private proxy routes.

Change status route to reflect its private use. Going forward, all such routes will be prefixed by /__sitecues-proxy/ to isolate them from the rest of the internet.

* Implement intelligent loader detection.

Some customers use website technologies that prevent them from adding attributes to elements or even using inline scripts. We now provide a variety of hints to inform the proxy that a loader is present.

* Handle SIGINT

* Document the process to create SSL infrastructure.

* Cleanup project metadata.

* Tweak legacy code.

* Create proper API and redesign CLI to use it.

* Singular directory names.

* Standardize dir name for private keys.

* Add SSL certificates.

* Ship the proxy CA with its private key.

Warning: This CA is for development purposes only, do not use it for anything public-facing. The private key should be considered insecure due to being online in version control and unecrypted. It is here to make development easier. To provide some level of protection, the proxy CA is an intermediate that can be revoked or recreated by the root CA when necessary. There be dragons.

* Enable HTTPS proxying.

* Use filter() for improved readability.

* Wire up the Try It for local HTTPS

Also fix missing dependency.

* Fix lint errors.

* Add Tidy lint automation and fix lint errors.

* Add Tidy lint automation and fix lint errors.

* Remove obsolete cruft: Grunt, JSCS, JSHint.

* Remove obsolete cruft: sanitizePort and test lists

* Fix lint errors.

* Bump deps and fix lint errors.

* Fix lint errors.

* Add Ci configuration.

* Unpin and bump deps. Solve some lint errors.

* Fix and simplify response handler.

* Fix intercept regressions.

* Unpin and bump dependencies.

* Extract response unzipping into new module.

* Extract shutdown code into new handle-quit module.

This is an extremely common pattern for servers. It deserves a life of its own.

* Move the new Try It to its own project.

* Bump dependencies and solve some lint errors.

* Remove obsolete code.

* Tweaks.

* Make the binary executable for ease of use.

* Simplify and remove cruft.

* Allow the user to set the log level.

Also solve some lint errors.

* Ensure the ca is included in package tarballs.

* Solve all lint errors.

* Hack around Hoxy’s bad exit behavior.

Hoxy has a server buried in its internals that it uses to accomplish some neat tricks for serving static files. Unfortunately, there’s no public way to shutdown this server. That conflicts with what we want to do to shutdown gracefully upon receiving SIGINT.

For now, we’re just going to force quit after a reasonable period.

Details: greim/hoxy#88

* General cleanup.

Improve comments, documentation, and error messages. Be smarter about “access denied” errors and don’t suggest to use sudo on Windows.

* Fix error handling for privileged ports.

* Revamp documentation.

* Fix CLI flag conflict.
@joaomilho
Copy link

joaomilho commented Aug 23, 2017

+1 having this problem. Simple example:

const hoxy = require('hoxy')

const proxy = hoxy.createServer().listen('1234')

proxy.close(function(err) {
  if (err) throw err

  console.log('The proxy is no longer accepting new connections.')
})

The output is The proxy is no longer accepting new connections., but my process is still up.

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

No branches or pull requests

2 participants