Skip to content

Add path http health check ability on before proxy start#95

Closed
le0pard wants to merge 2 commits into
basecamp:mainfrom
le0pard:health-check
Closed

Add path http health check ability on before proxy start#95
le0pard wants to merge 2 commits into
basecamp:mainfrom
le0pard:health-check

Conversation

@le0pard

@le0pard le0pard commented Oct 20, 2025

Copy link
Copy Markdown
Contributor

Motivation

Sometimes systems like Heroku start accept traffic if port ready in container. But in reality it is not correct, because rails may need more time to start. This PR introduce new config variable HTTP_HEALTH_PATH, which allow to set rails path, which should return 200 http status code. Only after returning 200 status code thruster start proxy on ports. In this case it allow to delay starting port before rails ready

Should fix #91

Comment thread internal/config.go
TargetPort: getEnvInt("TARGET_PORT", defaultTargetPort),
UpstreamCommand: os.Args[1],
UpstreamArgs: os.Args[2:],
UpstreamArgs: append([]string{}, os.Args[2:]...),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prevent UpstreamArgs mutation

@le0pard

le0pard commented Jan 27, 2026

Copy link
Copy Markdown
Contributor Author

Ping @monorkin @kevinmcconnell

@le0pard
le0pard force-pushed the health-check branch 2 times, most recently from 3877352 to 3975263 Compare February 7, 2026 09:08
@le0pard
le0pard force-pushed the health-check branch 2 times, most recently from 5d7033f to cc9d928 Compare February 12, 2026 21:32
@le0pard

le0pard commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

ping @monorkin @kevinmcconnell (maybe I need fix something in PR)

@kevinmcconnell

kevinmcconnell commented May 31, 2026

Copy link
Copy Markdown
Collaborator

@le0pard thinking about this more, I think a simpler approach would be to optionally delay binding the Thruster ports until the upstream port is listening. That would make a Thruster-enabled app start listening at the same point it would have done when not Thruster-enabled. And it avoids the need to configure health checks.

It should still be opt-in, because in some cases the behaviour of returning a 502 is useful (we use it in Campfire for a startup screen, for example). But we could have something like HTTP_BIND_DELAYED or similar, which opts in to waiting to bind the Thruster ports until the app port is accepting connections, and then people using Heroku could set that.

@le0pard

le0pard commented May 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for feedback @kevinmcconnell

Problem with delay it is not 100% predictable, so I need overcompensate and set bigger timeout for safety (which delay deploy) or sometimes have 500 error when deploy on heroku.

This patch version already working on production from 2025-11 and it works good. Just need to be sure, that endpoint return 200 http code by http (not https) protocol (that is why I added in readme how correctly to do this for rails). It also opt-in - by default healcheck is not active until developer not provided path.

Best solution if heroku support http check, so it will resolve issue automatically, but heroku in maintenance mode, so no new feature expected from platform

@kevinmcconnell

Copy link
Copy Markdown
Collaborator

@le0pard sorry, I didn’t mean a timeout. What I mean is, thruster can try to connect to the upstream port to find out when it is listening. However we don’t need to send it HTTP requests and get a 200, we just need to find out when that port is up.

That way the behaviour on Heroku would match whatever it does when Thruster isn’t involved, because the timing of the ports opening will match. And the check and configuration are simpler this way because all you need is a boolean flag for opting in to this behaviour.

@le0pard

le0pard commented May 31, 2026

Copy link
Copy Markdown
Contributor Author

Oh, ok, I can try to change code. I will ping when it is done

@le0pard

le0pard commented May 31, 2026

Copy link
Copy Markdown
Contributor Author

I'm going to close this PR in favor of a simpler approach: a TCP port readiness check - #130

Please check new PR @kevinmcconnell

@le0pard le0pard closed this May 31, 2026
@le0pard
le0pard deleted the health-check branch May 31, 2026 12:54
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

Successfully merging this pull request may close these issues.

Thruster on Heroku - Delay binding to HTTP_PORT until TARGET_PORT is available?

2 participants