Skip to content

feat(rust): add mergify-core::http with retry + typed errors (Phase 1.2b)#1281

Draft
jd wants to merge 1 commit intodevs/jd/worktree-rust-port/add-mergify-core-foundation-types-phase-1-2a--f50b81bbfrom
devs/jd/worktree-rust-port/add-mergify-core-http-retry-typed-errors-phase-1--9c66057c
Draft

feat(rust): add mergify-core::http with retry + typed errors (Phase 1.2b)#1281
jd wants to merge 1 commit intodevs/jd/worktree-rust-port/add-mergify-core-foundation-types-phase-1-2a--f50b81bbfrom
devs/jd/worktree-rust-port/add-mergify-core-http-retry-typed-errors-phase-1--9c66057c

Conversation

@jd
Copy link
Copy Markdown
Member

@jd jd commented Apr 21, 2026

Adds the HTTP client every ported command uses. Wraps reqwest
with the invariants the design requires:

  • Bearer-token auth injected when the token is non-empty; skipped
    otherwise so GitHub's anonymous public-repo reads still work.
  • Tenacity-style retry on 5xx and transient network errors: 3
    attempts, exponential backoff starting at 1s. 4xx responses are
    never retried — those are caller errors and retrying would mask
    bugs.
  • Typed error mapping: HTTP failures become either
    CliError::GitHubApi or CliError::MergifyApi based on the
    ApiFlavor passed at construction, so exit_code() routes
    to GITHUB_API_ERROR (5) or MERGIFY_API_ERROR (6)
    automatically.
  • Per-request timeout of 30s.
  • RetryPolicy is public so tests can skip the wall-clock
    backoff with initial_backoff = 0.

Also extends CliError with the two new API variants and
updates the exit_code() mapping + unit tests.

Command crates MUST NOT import reqwest directly — the only
HTTP surface is HttpClient::get and HttpClient::post.

6 new tests cover: happy-path GET, empty-token behavior, POST
with JSON body, retry on 5xx then success, exhausted retries
mapping to MergifyApi, 4xx not retried and mapping to
GitHubApi. All use wiremock for hermetic local HTTP.

Next sub-phase (1.2c / 1.2d) adds colored/spinner affordances and
the GitOps trait only when the first ported command needs
them; this PR is the minimum config validate requires.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

Depends-On: #1280

@jd
Copy link
Copy Markdown
Member Author

jd commented Apr 21, 2026

This pull request is part of a Mergify stack:

# Pull Request Link
1 feat(rust): scaffold Cargo workspace and Rust CI #1263
2 feat(rust): add mergify-py-shim with embedded Python fallback #1272
3 feat(rust): add mergify-core foundation types (Phase 1.2a) #1280
4 feat(rust): add mergify-core::http with retry + typed errors (Phase 1.2b) #1281 👈
5 feat(rust): port config validate to native Rust (Phase 1.3) #1282

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Apr 21, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🔴 👀 Review Requirements

Waiting for:

  • #approved-reviews-by>=2
This rule is failing.
  • any of:
    • #approved-reviews-by>=2
    • author = dependabot[bot]
    • author = mergify-ci-bot
    • author = renovate[bot]

🔴 🔎 Reviews

Waiting for:

  • #review-requested = 0
This rule is failing.
  • #review-requested = 0
  • #changes-requested-reviews-by = 0
  • #review-threads-unresolved = 0

🟢 🤖 Continuous Integration

Wonderful, this rule succeeded.
  • all of:
    • check-success=ci-gate

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?:

🟢 📕 PR description

Wonderful, this rule succeeded.
  • body ~= (?ms:.{48,})

….2b)

Adds the HTTP client every ported command uses. Wraps ``reqwest``
with the invariants the design requires:

- Bearer-token auth injected when the token is non-empty; skipped
  otherwise so GitHub's anonymous public-repo reads still work.
- Tenacity-style retry on 5xx and transient network errors: 3
  attempts, exponential backoff starting at 1s. 4xx responses are
  never retried — those are caller errors and retrying would mask
  bugs.
- Typed error mapping: HTTP failures become either
  ``CliError::GitHubApi`` or ``CliError::MergifyApi`` based on the
  ``ApiFlavor`` passed at construction, so ``exit_code()`` routes
  to ``GITHUB_API_ERROR`` (5) or ``MERGIFY_API_ERROR`` (6)
  automatically.
- Per-request timeout of 30s.
- ``RetryPolicy`` is public so tests can skip the wall-clock
  backoff with ``initial_backoff = 0``.

Also extends ``CliError`` with the two new API variants and
updates the ``exit_code()`` mapping + unit tests.

Command crates MUST NOT import ``reqwest`` directly — the only
HTTP surface is ``HttpClient::get`` and ``HttpClient::post``.

6 new tests cover: happy-path GET, empty-token behavior, POST
with JSON body, retry on 5xx then success, exhausted retries
mapping to ``MergifyApi``, 4xx not retried and mapping to
``GitHubApi``. All use ``wiremock`` for hermetic local HTTP.

Next sub-phase (1.2c / 1.2d) adds colored/spinner affordances and
the ``GitOps`` trait only when the first ported command needs
them; this PR is the minimum ``config validate`` requires.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Change-Id: I9c66057cc53acd0098f43086352ca9a9bf5aa7b0
@jd jd force-pushed the devs/jd/worktree-rust-port/add-mergify-core-http-retry-typed-errors-phase-1--9c66057c branch from 61966dc to 0e72eb6 Compare April 21, 2026 13:13
@jd
Copy link
Copy Markdown
Member Author

jd commented Apr 21, 2026

Revision history

# Type Changes Date
1 initial 61966dc 2026-04-21 13:13 UTC
2 rebase 61966dc → 0e72eb6 2026-04-21 13:13 UTC

@mergify mergify Bot had a problem deploying to Mergify Merge Protections April 21, 2026 13:14 Failure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant