Skip to content

refactor: extract shared probehttp helper; replace http.DefaultClient#41

Merged
Patel230 merged 2 commits into
mainfrom
refactor/probehttp-shared-helper
Jun 18, 2026
Merged

refactor: extract shared probehttp helper; replace http.DefaultClient#41
Patel230 merged 2 commits into
mainfrom
refactor/probehttp-shared-helper

Conversation

@Patel230

Copy link
Copy Markdown
Contributor

Extracts the credential-probe / model-probe HTTP plumbing and status-code-to-error mapping into internal/probehttp. Replaces three uses of http.DefaultClient (no Timeout) with a shared 15s-timeout client. Dedupes probeStatusErr (catalog/xiaomi) and probeHTTPError (config/credential), which were near-identical with the same error messages.

Adds 7 tests for the new package and keeps the public API stable for hawk.

Verification: go vet, staticcheck, gofumpt, go build, govulncheck, go test -short ./... all clean; go test -race on the affected packages clean.

Patel230 added 2 commits June 18, 2026 03:41
The eyrie repo had three credential-probe / model-probe sites that
each used http.DefaultClient.Do() and each hand-rolled the same
status-code-to-error mapping. Three problems with that:

  1. http.DefaultClient has no Timeout, so a slow or hijacked
     response can hang the probe forever. The call sites *do* wrap
     the request in a context, so the in-flight request itself will
     be cancelled, but TLS / dial / redirect / response-header
     reads before the body is opened are not bounded.

  2. probeStatusErr (catalog/xiaomi) and probeHTTPError
     (config/credential) were near-duplicates with identical switch
     cases and identical error messages. A bug fix in one would
     silently miss the other.

  3. probe-specific plumbing (request build, body drain, status
     check) was reimplemented per call site, which made the probe
     functions hard to read.

This change introduces internal/probehttp:

  - DefaultClient: a shared *http.Client with a 15s Timeout.
  - ProbeError(status): single source of truth for the
    HTTP-status-to-error mapping. Stable error wording (those
    strings are part of what hawk surfaces to users on /config
    probe failure).
  - DoGet(ctx, url, headers): builds a GET, applies headers, runs
    through DefaultClient, drains up to 1 MiB of body, returns
    (status, body, err).
  - UserAgent(), JoinURL(): small shared helpers so call sites
    stop re-implementing the trim/concat dance.

The two consumer packages now import probehttp and the call sites
collapse from 6 functions (probeOpenAIModels, probeAnthropic,
probeGemini, doProbeRequest, probeHTTPError, plus the xiaomi-side
doProbe/doModelsRequest/probeStatusErr) to 4 thin wrappers
(probeOpenAIModels, probeAnthropic, probeGemini in
config/credential, and the two exported entry points in
catalog/xiaomi) that each issue a DoGet and translate the status
via ProbeError.

Verification:
  - go build ./...                -> clean
  - go vet ./...                  -> clean
  - staticcheck ./...             -> clean
  - gofumpt -d                    -> clean
  - go test -short ./...          -> all packages pass
  - go test ./internal/probehttp  -> 8 new tests cover
                                       ProbeError, DoGet
                                       (success + body bound
                                       + context deadline), and
                                       the URL/UA helpers.
Remove the trailing blank lines left over from removing the
TestDoGet_NilContextDoesNotPanic test in the previous commit.

Verification:
  - gofumpt -l .                -> 0 files
  - go test ./internal/probehttp -> all pass
@Patel230 Patel230 merged commit a7e62a3 into main Jun 18, 2026
15 checks passed
@Patel230 Patel230 deleted the refactor/probehttp-shared-helper branch June 18, 2026 04:08
Patel230 added a commit that referenced this pull request Jun 18, 2026
…#41)

* refactor: extract shared probehttp helper; replace http.DefaultClient

The eyrie repo had three credential-probe / model-probe sites that
each used http.DefaultClient.Do() and each hand-rolled the same
status-code-to-error mapping. Three problems with that:

  1. http.DefaultClient has no Timeout, so a slow or hijacked
     response can hang the probe forever. The call sites *do* wrap
     the request in a context, so the in-flight request itself will
     be cancelled, but TLS / dial / redirect / response-header
     reads before the body is opened are not bounded.

  2. probeStatusErr (catalog/xiaomi) and probeHTTPError
     (config/credential) were near-duplicates with identical switch
     cases and identical error messages. A bug fix in one would
     silently miss the other.

  3. probe-specific plumbing (request build, body drain, status
     check) was reimplemented per call site, which made the probe
     functions hard to read.

This change introduces internal/probehttp:

  - DefaultClient: a shared *http.Client with a 15s Timeout.
  - ProbeError(status): single source of truth for the
    HTTP-status-to-error mapping. Stable error wording (those
    strings are part of what hawk surfaces to users on /config
    probe failure).
  - DoGet(ctx, url, headers): builds a GET, applies headers, runs
    through DefaultClient, drains up to 1 MiB of body, returns
    (status, body, err).
  - UserAgent(), JoinURL(): small shared helpers so call sites
    stop re-implementing the trim/concat dance.

The two consumer packages now import probehttp and the call sites
collapse from 6 functions (probeOpenAIModels, probeAnthropic,
probeGemini, doProbeRequest, probeHTTPError, plus the xiaomi-side
doProbe/doModelsRequest/probeStatusErr) to 4 thin wrappers
(probeOpenAIModels, probeAnthropic, probeGemini in
config/credential, and the two exported entry points in
catalog/xiaomi) that each issue a DoGet and translate the status
via ProbeError.

Verification:
  - go build ./...                -> clean
  - go vet ./...                  -> clean
  - staticcheck ./...             -> clean
  - gofumpt -d                    -> clean
  - go test -short ./...          -> all packages pass
  - go test ./internal/probehttp  -> 8 new tests cover
                                       ProbeError, DoGet
                                       (success + body bound
                                       + context deadline), and
                                       the URL/UA helpers.

* style: gofumpt-cleanup of probehttp_test.go

Remove the trailing blank lines left over from removing the
TestDoGet_NilContextDoesNotPanic test in the previous commit.

Verification:
  - gofumpt -l .                -> 0 files
  - go test ./internal/probehttp -> all pass
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.

1 participant