fix: supervise the cowboy listener instead of starting it as a side effect#389
Open
Taure wants to merge 1 commit into
Open
fix: supervise the cowboy listener instead of starting it as a side effect#389Taure wants to merge 1 commit into
Taure wants to merge 1 commit into
Conversation
…ffect nova_sup started cowboy via cowboy:start_clear/start_tls inside init/1 and discarded the result. A failed bind (e.g. eaddrinuse) was only logged, so application:start(nova) still returned ok with no working listener and requests silently hit whatever already held the port. Build the listener with ranch:child_spec/5 and put it in the supervision tree. A bind failure now fails init/1 and surfaces through application:start/1, and the listener is restart-managed like any other child. The option transforms mirror cowboy:start_clear/start_tls (2.15) so buffer/connection_type behaviour is identical. Adds nova_sup_tests covering the child-spec shape and that a busy port fails the start instead of being swallowed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
nova_sup:init/1starts cowboy by callingcowboy:start_clear/3(orstart_tls/3) as a side effect and discards the result:setup_cowboy/1logs{error, Reason}and returns. So when the listener can't bind (e.g. another node already holds the port,eaddrinuse), nova logsCowboy could not startbutapplication:start(nova)still returns{ok, ...}. The node comes up "healthy" with no working listener, and requests silently hit whatever already owns the port.This is a nasty footgun in development (a stale
make start/shell node holding 8080) and in production (port conflict on deploy):ensure_all_started(my_app)succeeds, health checks may pass, but no route resolves. The symptom looks exactly like a routing bug (manualrouting_tree:lookupreturns{ok, ...}in the new node, but the request 404s because it landed on the other node).Fix
Build the listener with
ranch:child_spec/5and add it to nova's supervision tree instead of starting it as a side effect:init/1and surfaces throughapplication:start/1(loud, not swallowed).The transport/protocol option transforms (
connection_type, dynamic buffer) mirrorcowboy:start_clear/start_tlsfor cowboy 2.15 (the pinned version) so socket behaviour is identical to before.Before / after
Tests
test/nova_sup_tests.erl:child_spec_is_supervised_test/tls_child_spec_uses_ssl_transport_test— the listener is asupervisor:child_spec()withtype => supervisorandconnection_type => supervisor.listener_lifecycle_test_— a free port yields a live listener; a busy port fails the start instead of being swallowed.Full suite green:
eunit351/0,xrefclean,dialyzerclean. Also dogfooded through a downstream Nova app: the listener-bind failure now correctly aborts that app's startup.