Audit remediation: docs correctness, TLS safe-by-default, narrowed error handling#8
Merged
Conversation
…scues From a /phx:audit health check. Three groups of fixes: Docs (H1/H2/L2/L3): - Rewrite README examples that called a non-existent GrpcConnectionPool.execute/1 to the real get_channel/1 + Stub.call pattern; no new API. - Fix stale install snippet in the GrpcConnectionPool moduledoc (0.1.0/0.10.2 -> 0.3/0.11). - Document telemetry_sample_rate and the full pool option set in README + Config moduledoc; correct the :get_channel telemetry row (empty measurements) and the PowerOfTwo "least-recently-used" -> "least-frequently-used". - mix.exs @Version 0.3.0 -> 0.3.5 (match the released tag); add CHANGELOG [0.3.5] + [Unreleased]; fix 0.2.2/0.2.3 date typos (2025 -> 2026). Security (M1/M2): - Config.production/1 now defaults to verifying TLS on OTP 25+ (verify_peer + system cacerts + hostname check); warns and falls back on older OTP. Exposed as Config.default_production_ssl/0. - An *explicit* `type: :production` endpoint built without :ssl/:credentials now raises instead of silently connecting over plaintext h2c. Defaulted type stays permissive for local/test use. - Tests for both, plus updated the production+interceptors test to use TLS. Robustness (L4/L5/L6): - Pool.scale_up/scale_down/resize rescue ArgumentError -> {:error, :pool_not_found}; other exceptions propagate instead of being returned as opaque structs. - TelemetryReporter logs a warning before recovering instead of swallowing errors silently. - Worker.send_ping/1 drops the broad rescue/catch (the body is total). - Demote expected reconnect/retry logs to :debug; gate the connection-failure error log on suppress_connection_errors.
Adds a test that starts an in-process GRPC server (reusing a service from the
test-only googleapis_proto_ex, port 0 = OS-assigned) and points a pool at it,
asserting workers connect, all slots populate (current_size == pool_size,
status :healthy), and get_channel/1 returns a real %GRPC.Channel{}. Covers the
worker -> GRPC.Stub.connect -> slot claim -> get_channel path in CI without the
external Pub/Sub emulator. Stable across repeated seeded runs.
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.
Summary
Remediation of every finding from a project health audit (
/phx:audit).Three groups of changes — documentation correctness, security hardening,
and error-handling/robustness — plus a new test that closes a real
connect-path coverage gap. No hot-path or architecture changes.
Reviewable commit-by-commit:
d9e0a84b0c2925Documentation
GrpcConnectionPool.execute/1. Themodule exposes
get_channel/1(+ start/stop/scale/status), notexecute.Anyone following the README's main "call a service" examples hit
UndefinedFunctionError. Rewrote all of them (lines 98/492/514–515/526/846) tothe real
get_channel/1+Stub.callpattern. No new API.GrpcConnectionPoolmoduledoc(
~> 0.1.0/grpc ~> 0.10.2→~> 0.3/~> 0.11).telemetry_sample_rateand the full pool option set(README +
Configmoduledoc); corrected the:get_channeltelemetry row(emits no measurements, not
duration) and the PowerOfTwo description("least-recently-used" → "least-frequently-used", lock-free atomics).
mix.exs @version0.3.0→0.3.5to match the released tag; addedCHANGELOG
[0.3.5]and[Unreleased]sections.0.2.2/0.2.3CHANGELOG date typos (2025-01-29→2026-01-29).Security (⚠️ behavior change)
type: :productionendpoint built without:sslor:credentialsnow raises instead of silently connecting over plaintexth2c. Scoped to explicit production so defaulted/
:localconfigs (and existinglocal/test usage) are unaffected.
Config.production/1now defaults to verifying TLS on OTP 25+(
verify_peer+ system CA store + hostname check via:public_key.pkix_verify_hostname_match_fun/1); on OTP 24 (no system-CAaccessor) it warns and falls back, prompting an explicit
:ssl. Exposed asConfig.default_production_ssl/0.use TLS.
Robustness
Pool.scale_up/scale_down/resizerescues narrowed:ArgumentError(pool not started / ETS missing) →
{:error, :pool_not_found}; any otherexception now propagates instead of being swallowed as an opaque struct.
TelemetryReporterlogs a warning before recovering instead of swallowingsilently.
Worker.send_ping/1's blanketrescue _ / catch _(the bodyis total), so a future programming error surfaces instead of masquerading as a
ping failure.
:debugand gated theconnection-failure
:errorlog onsuppress_connection_errors(reducesreconnect-storm log spam).
Tests
connect_path_test.exsstarts an in-process gRPC server(reusing a service from the test-only
googleapis_proto_ex, port0=OS-assigned) and points a pool at it, asserting workers connect, all slots
populate (
current_size == pool_size, status:healthy), andget_channel/1returns a real
%GRPC.Channel{}. Covers worker →GRPC.Stub.connect→ slotclaim →
get_channelin CI without the external Pub/Sub emulator. Stableacross repeated seeded runs.
Verification
mix compile --warnings-as-errors— cleanmix format --check-formatted— cleanmix credo --strict— no issuesmix test— 91 passed, 3 excluded (emulator-tagged)