Skip to content

Fix issues found during the connection-reuse review - #13

Merged
jonathan-kosgei merged 4 commits into
mainfrom
fix-noticed-issues
Nov 3, 2026
Merged

jonathan-kosgei merged 4 commits into
mainfrom
fix-noticed-issues

Conversation

@jonathan-kosgei

@jonathan-kosgei jonathan-kosgei commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #12. It fixes the items recorded under "Noticed, not changed" in that pull request that are stale documentation or CI configuration. The library's code and public API are untouched: the changes are confined to the README and the GitHub Actions workflow.

Changes

  • README, "Connection reuse": the handshake timing figures are gone. The section now states that a new connection pays a TCP and TLS handshake and a request on a reused connection does not, without quoting numbers that depend on the reader's network.
  • Workflow name: CI instead of "Publish to Hex.pm". The workflow has run the test suite on pull requests and pushes to main since Reuse HTTPS connections, add new response fields #12, so the checks on pull requests and the Actions tab are now labelled accordingly; the publish job keeps its name and its v* tag gate.
  • fail-fast: false on the test matrix: a failing OTP release no longer cancels the other jobs, so every release reports its own result instead of showing as cancelled.
  • actions/checkout@v7 and actions/cache@v6 replace the v4 releases, which the runner flagged as targeting the deprecated Node.js 20 (both current majors run on Node.js 24; erlef/setup-beam@v1 already does). The deprecation warning disappears from the job logs.
  • rebar3 version: instead of one 3.24 pin for every job, each OTP release is paired with the newest rebar3 release that supports it according to rebar3's compatibility table (3.24 for OTP 25, 3.25 for OTP 26, 3.28 for OTP 27 and later); the publish job uses OTP 27 with rebar3 3.28. The cache key includes the rebar3 version so a build directory is never restored into a job that runs a different rebar3.
  • Test matrix: OTP 28 is added; OTP 25, 26 and 27 stay.
  • README, "Look Up a Specific IP": the example values now match the API's response for 8.8.8.8 (an anycast address located at country level only: city is null, the coordinates are country-level, is_datacenter and is_anycast are true) instead of the Ashburn/Virginia values, and the example explains that fields without data are returned as null.

Not changed

  • httpc:request/5 raises exit:{noproc, _} instead of returning {error, {not_started, Profile}} for a profile that is not running: an OTP bug (httpc:handle_request/9 catches error:{noproc, _} while gen_server:call/3 exits), so the library keeps handling both shapes.
  • Retry of a bulk POST on {error, socket_closed_remotely}: httpc itself re-dispatches a request whose keep-alive connection was closed before the response headers arrived; the single retry in ipdata_http:post/4 covers the remaining case and stays as documented there.
  • TCP keepalive interval: httpc exposes no option for it, so the OTP defaults remain.
  • Publish job gating: the job-level if: startsWith(github.ref, 'refs/tags/v') is the standard form and shows the job as skipped on non-tag events; nothing to change.
  • OTP 29 is not in the matrix: the test dependency meck 0.9.2 does not compile there (its bare catch is deprecated in OTP 29 and the dependency build fails on that warning); meck 1.1.1 fixed it, but meck 1.1.0 dropped OTP 25, which the matrix keeps.
  • User-Agent: the client already sends erlang-ipdata/<application version> with the version read from the .app file at run time; the established header value was kept rather than renamed.

Verification

Run locally on OTP 25.3.2 with rebar3 3.19.0, with jsx 3.1.0 and meck 0.9.2 provided as _checkouts, before each push:

Command Result
rebar3 compile ok
rebar3 ct All 47 tests passed. (ipdata_SUITE 19, ipdata_app_SUITE 4, ipdata_http_SUITE 24)
rebar3 xref no issues
rebar3 dialyzer no warnings

The workflow file was additionally checked to parse as YAML with the expected name, triggers, fail-fast, matrix entries and uses references.

CI on this pull request:

  • Run 41639460791 on bea389d (matrix OTP 25 to 29): the OTP 25, 26, 27 and 28 jobs passed compile, rebar3 ct, rebar3 dialyzer and rebar3 xref, with no Node.js deprecation warning in the logs; the OTP 29 job failed in rebar3 ct while compiling meck 0.9.2 (see "Not changed"). Thanks to fail-fast: false the other four jobs ran to completion instead of being cancelled. The publish job was skipped (not a tag).
  • Run 41639852733 on 6b407a4 (OTP 29 removed): success. All four jobs (OTP 25 with rebar3 3.24 on ubuntu-24.04, OTP 26 with 3.25, OTP 27 with 3.28, OTP 28 with 3.28) passed; the publish job was skipped (not a tag).

OTP 26 to 28 are covered by CI only. The publish job can only be exercised by a v* tag.

🤖 Generated with Claude Code

The Connection reuse section quoted latency figures for a cold and a
reused connection. They depend on the network path between the client
and the API and go stale, so the section now only states that a new
connection pays a TCP and TLS handshake and a request on a reused
connection does not.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The workflow has run the test suite on pull requests and on pushes to
main since the connection-reuse change, but it was still named after
its publish job. It is now named CI; the publish job keeps its name
and its tag gate.

- fail-fast is off, so a failing OTP release no longer cancels the
  other jobs and every release reports its own result.
- actions/checkout and actions/cache move to their current majors,
  which run on Node.js 24; the runner had been warning that the v4
  releases target the deprecated Node.js 20.
- Each OTP release is paired with the newest rebar3 release that
  supports it (3.24 for OTP 25, 3.25 for OTP 26, 3.28 for OTP 27 and
  later) instead of one 3.24 pin for all of them, and the cache key
  includes the rebar3 version so a build directory is never shared
  between rebar3 releases.
- OTP 28 and 29 join the test matrix; the publish job stays on OTP 27,
  now with rebar3 3.28.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The "Look Up a Specific IP" example showed a city, city-level
coordinates and a time zone for 8.8.8.8 that the API does not return
for that address: it is an anycast address located at country level
only, so city is null and the coordinates are country-level. The
example now shows the values of the API's own sample response for the
address (the same sample serves as a test fixture), explains that
fields without data are returned as null, and shows the datacenter
flag the address does carry.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
meck 0.9.2, the test dependency, does not compile on OTP 29: its
meck_matcher module uses a bare catch, which OTP 29 deprecates, and
the dependency build fails on that warning. meck 1.1.1 replaced the
catch, but meck 1.1.0 dropped OTP 25, which this matrix keeps, so the
matrix stops at OTP 28 for now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@jonathan-kosgei
jonathan-kosgei merged commit de63d11 into main Nov 3, 2026
5 checks passed
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.

2 participants