Replace reqwest with a direct hyper transport

[leynos]

Summary

This branch delivers PR 2 of the three-phase GitHub API modernisation
governed by
ADR 001:
the bespoke GraphQL client now sends requests through a direct hyper
transport, and reqwest leaves the dependency graph entirely. The binary
converges on one HTTP stack (hyper/rustls), shared with octocrab.

Execplan: docs/execplans/adopt-octocrab.md
— PR 2 of its three phases lands here. Stacked on #194 (PR 1, octocrab
REST); the typed-GraphQL phase (PR 3) remains as authorized future work.

Observable behaviour is unchanged: transcript format, retry semantics and
attempt counts, error-message text, header set, and the
GITHUB_GRAPHQL_URL endpoint override all survive, evidenced by the full
suite passing with no test edits (one inline test module swapped a
reqwest::header import for http::header — the same types reqwest
re-exported).

Review walkthrough

• Start with
  src/api/client/transport.rs
  — a pooled hyper-util legacy client with a hyper-rustls connector
  (webpki roots + ring, matching what reqwest's rustls-tls feature
  provided; https_or_http keeps loopback test servers working). One
  tokio::time::timeout spans send plus body collection, mirroring
  reqwest's .timeout() scope; timeouts map to
  VkError::RequestContext, which the retry classifier already treats
  as transient. System proxies and redirects are deliberately
  unsupported and documented.
• Then
  src/api/client/mod.rs
  — GraphQLClient holds a Transport instead of a reqwest::Client;
  transcript logging, status handling, and body-snippet error context
  are untouched.
• Check
  Cargo.toml
  — reqwest removed; hyper, hyper-util, hyper-rustls, and
  http-body-util promoted to runtime dependencies, unifying former
  dev-only entries.
• Finish with
  docs/vk-end-to-end-testing-guide.md
  — corrects a long-standing inaccuracy: the harness is plain hyper
  loopback stubs driven by env-var endpoint overrides, not a
  man-in-the-middle proxy.

Validation

• make check-fmt / make lint / make test: pass (274 tests, 1 ignored)
• cargo test --test e2e -- --ignored e2e_pr_42: pass (transcript-format parity replayed from the recorded fixture)
• cargo tree -i reqwest (also -e normal and --all-features): package not found
• make markdownlint / make nixie: pass
• coderabbit review --agent: completed, zero findings

Notes

• The binary-internal VkError::Request variant (constructible only from
  reqwest errors) was removed with its retry-classifier arm; VkError is
  not exported from src/lib.rs, so the library surface is unchanged.
• reqwest honoured HTTP(S)_PROXY and followed redirects by default; the
  new transport does neither. Both behaviours are unused and untested on
  the GraphQL path and are documented in the transport module and the
  ExecPlan decision log.

[sourcery-ai]

Sorry @leynos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dd01bf2e-5e03-4bf6-b3be-43036205e2a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch hyper-graphql-transport

---

_{Comment @coderabbitai help to get the list of available commands.}

[codescene-access]

Gates Failed
Enforce advisory code health rules (1 file with Excess Number of Function Arguments)

Our agent can fix these. Install it.

Gates Passed
5 Quality Gates Passed

Reason for failure

Enforce advisory code health rules	Violations	Code Health Impact
transport.rs	1 advisory rule	9.69	Suppress

See analysis details in CodeScene

Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

[coderabbitai]

Oops, something went wrong! Please try again later. 🐰 💔

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

src/api/client/transport.rs

Comment on lines +86 to +151

    pub(super) async fn post_json(
        &self,
        endpoint: &Endpoint,
        headers: &HeaderMap,
        payload: &Value,
        timeout: Duration,
    ) -> Result<HttpResponse, VkError> {
        // Reconstruct the same request-context strings the reqwest client
        // produced: the operation name (or a query snippet fallback) and a
        // redacted payload snippet. Derived from the payload so this transport
        // stays self-contained behind its fixed signature.
        let snip = payload_snippet(payload);
        let query = payload.get("query").and_then(Value::as_str).unwrap_or("");
        let operation = operation_name(query).map_or_else(|| snippet(query, 64), str::to_string);
        let make_ctx = |status: Option<u16>| {
            let base = format!("operation {operation}; {snip}");
            // Disambiguate from `BodyExt::boxed`, which is also in scope.
            BoxedStr::boxed(match status {
                Some(s) => format!("{base}; status {s}"),
                None => base,
            })
        };

        let request = build_request(endpoint, headers, payload).map_err(|source| {
            VkError::RequestContext {
                context: make_ctx(None),
                source,
            }
        })?;

        let exchange = async {
            let response =
                self.client
                    .request(request)
                    .await
                    .map_err(|e| VkError::RequestContext {
                        context: make_ctx(None),
                        source: Box::new(e),
                    })?;
            let status = response.status().as_u16();
            let collected =
                response
                    .into_body()
                    .collect()
                    .await
                    .map_err(|e| VkError::RequestContext {
                        context: make_ctx(Some(status)),
                        source: Box::new(e),
                    })?;
            // Lossily decode, matching reqwest's UTF-8 text handling; decoding
            // never fails, so only a collection error can surface above.
            let body = String::from_utf8_lossy(&collected.to_bytes()).into_owned();
            Ok::<HttpResponse, VkError>(HttpResponse { status, body })
        };

        match tokio::time::timeout(timeout, exchange).await {
            Ok(result) => result,
            Err(_elapsed) => Err(VkError::RequestContext {
                context: make_ctx(None),
                source: Box::new(std::io::Error::new(
                    std::io::ErrorKind::TimedOut,
                    format!("request timed out after {timeout:?}"),
                )),
            }),
        }
    }

❌ New issue: Excess Number of Function Arguments
Transport.post_json has 5 arguments, max arguments = 4

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
fd -a 'transport.rs' src/api/client || true
echo "---outline---"
ast-grep outline src/api/client/transport.rs --items all 2>/dev/null | head -100
echo "---full file---"
sed -n '1,220p' src/api/client/transport.rs

Length of output: 8705

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "---mod.rs outline---"
ast-grep outline src/api/client/mod.rs --items all 2>/dev/null | head -80
echo "---callers of post_json---"
rg -n 'post_json' src -A5 -B5

Length of output: 2663

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '100,145p' src/api/client/mod.rs

Length of output: 1837

---

@leynos here's the fix for Transport::post_json in src/api/client/transport.rs.

Bundle the four data arguments into a request struct so post_json takes &self plus one parameter instead of &self plus four. The sole call site is GraphQLClient::execute_single_request in src/api/client/mod.rs (lines 130–138), which is a mechanical one-line change.

Introduce a PostJsonRequest<'a> struct alongside Transport in src/api/client/transport.rs:

/// Bundles the per-request inputs to [`Transport::post_json`].
pub(super) struct PostJsonRequest<'a> {
    pub(super) endpoint: &'a Endpoint,
    pub(super) headers: &'a HeaderMap,
    pub(super) payload: &'a Value,
    pub(super) timeout: Duration,
}

Change the method signature and destructure at the top of the body:

pub(super) async fn post_json(
    &self,
    request: PostJsonRequest<'_>,
) -> Result<HttpResponse, VkError> {
    let PostJsonRequest { endpoint, headers, payload, timeout } = request;
    // ... rest of the body is unchanged, referring to `endpoint`, `headers`,
    // `payload`, and `timeout` as before.

Update the call site in src/api/client/mod.rs:

let resp = self
    .transport
    .post_json(transport::PostJsonRequest {
        endpoint: &self.endpoint,
        headers: &self.headers,
        payload,
        timeout: self.retry.request_timeout,
    })
    .await?;

(Export PostJsonRequest from the transport module — it's already pub(super), so no visibility change needed beyond the struct itself; mod.rs already has mod transport; in scope.)

This drops post_json to two effective arguments (self, request), satisfying CodeScene's threshold, without touching the transcript, retry, or error-context logic that the PR explicitly preserves.

---

Prompt for an AI coding agent

Refactor `Transport::post_json` in `src/api/client/transport.rs` (vk repository) to
resolve a CodeScene "Excess Number of Function Arguments" finding (5 arguments,
max 4).

1. In `src/api/client/transport.rs`, add a new struct near the top of the file
   (after the `Transport` struct definition, before `impl Transport`):

   ```rust
   /// Bundles the per-request inputs to [`Transport::post_json`].
   pub(super) struct PostJsonRequest<'a> {
       pub(super) endpoint: &'a Endpoint,
       pub(super) headers: &'a HeaderMap,
       pub(super) payload: &'a Value,
       pub(super) timeout: Duration,
   }
   ```

2. Change the signature of `Transport::post_json` from:

   ```rust
   pub(super) async fn post_json(
       &self,
       endpoint: &Endpoint,
       headers: &HeaderMap,
       payload: &Value,
       timeout: Duration,
   ) -> Result<HttpResponse, VkError> {
   ```

   to:

   ```rust
   pub(super) async fn post_json(
       &self,
       request: PostJsonRequest<'_>,
   ) -> Result<HttpResponse, VkError> {
       let PostJsonRequest { endpoint, headers, payload, timeout } = request;
   ```

   Keep the rest of the method body unchanged — it already references
   `endpoint`, `headers`, `payload`, and `timeout` by name.

3. Update the doc comment above `post_json` to describe the single `request`
   parameter instead of the four separate ones (keep the existing `# Errors`
   section intact).

4. In `src/api/client/mod.rs`, update the sole call site inside
   `GraphQLClient::execute_single_request` from:

   ```rust
   let resp = self
       .transport
       .post_json(
           &self.endpoint,
           &self.headers,
           payload,
           self.retry.request_timeout,
       )
       .await?;
   ```

   to:

   ```rust
   let resp = self
       .transport
       .post_json(transport::PostJsonRequest {
           endpoint: &self.endpoint,
           headers: &self.headers,
           payload,
           timeout: self.retry.request_timeout,
       })
       .await?;
   ```

   Confirm `transport` is already imported as a module (it is, via
   `mod transport;` at the top of `mod.rs`) so `transport::PostJsonRequest` is
   directly reachable; adjust the path if the module is aliased differently.

5. Run `cargo build` and `cargo test` (or the repository's `make check-fmt`,
   `make lint`, `make test` targets) to confirm the change compiles and all
   274 existing tests still pass, with no behavioural change to transcript
   logging, retry semantics, or error-context strings.

Do not change any other logic inside `post_json`'s body, the timeout handling,
or the error-context construction (`make_ctx`); this is purely a
parameter-bundling refactor.

---

Suppression rationale (if you choose not to refactor)

If you decide the struct-bundling refactor is not worth the churn for a single call site, paste this into CodeScene's suppression field:

Transport::post_json takes five arguments (&self, endpoint, headers, payload, timeout) because it deliberately reconstructs, in one self-contained call, exactly the inputs reqwest previously received implicitly via a pre-configured reqwest::Client and request builder chain. It has exactly one call site (GraphQLClient::execute_single_request), all four data parameters are independently meaningful (a fixed GraphQL endpoint, per-request auth headers, the JSON payload, and a per-call timeout), and none of them naturally group into a pre-existing domain type without introducing a purpose-built wrapper struct solely to satisfy this metric. Splitting the method further would fragment request construction, timeout scoping, and error-context derivation (make_ctx) across multiple functions, working against the single-timeout-spans-everything design documented at the top of transport.rs and increasing, not decreasing, the surface for a reviewer to hold in mind at once.

That said, the struct-based fix above is low-risk and mechanical (one struct, one call site), so I'd recommend applying it rather than suppressing, unless you have a specific reason to keep the flat signature.

---

✏️ Learnings added

Learnt from: leynos
URL: https://github.com/leynos/vk/pull/195

Timestamp: 2026-07-09T15:35:00.212Z
Learning: In the vk repository (Rust), CodeScene's "Excess Number of Function Arguments" check counts `&self` as one of the arguments, so a method with `&self` plus 4 parameters (5 total) already exceeds the max-4-arguments threshold.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.