Skip to content

Fix RESTWithNext error type, repairing gh status and attestation retries - #13988

Merged
williammartin merged 1 commit into
trunkfrom
williammartin-fix-restwithnext-error-type
Aug 1, 2026
Merged

Fix RESTWithNext error type, repairing gh status and attestation retries#13988
williammartin merged 1 commit into
trunkfrom
williammartin-fix-restwithnext-error-type

Conversation

@williammartin

@williammartin williammartin commented Jul 28, 2026

Copy link
Copy Markdown
Member

Description

The api.Client.RESTWithNext method did not handle errors in the same way as api.Client.REST. The latter wrapped the go-gh error into a new api error and the former simply returned it. From an error message point of view, there was no visible difference but programmatic checks for errors.As(err, &api.HTTPError{}) would be true for REST and false for RESTWithNext.

I cannot find any reason for this to have been intentional. I suspect what happened was that RESTWithNext previously used to handle this case via the following removed code:

	success := resp.StatusCode >= 200 && resp.StatusCode < 300
	if !success {
		return "", HandleHTTPError(resp)
	}

But at some point go-gh was created, and the go-gh api client Request/RequestWithContext methods began returning errors for non success http status codes.

This broke two behaviours:

  • gh status is supposed to gracefully degrade if notifications or events are inaccessible.
  • gh attestation commands is supposed to retry attestation fetching for 500 status codes.

Notes

This is a prerequisite for consolidating GitHub API URL derivation onto api.Client (part of the groundwork for per-host API routing, #13717 / #13760), but it is a user-facing bugfix in its own right and is worth reviewing as such.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes api.Client.RESTWithNext to return the GitHub CLI’s api.HTTPError (via handleResponse) instead of raw go-gh HTTP errors, restoring status-code inspection (errors.As) and ensuring retry logic works consistently across REST helpers.

Changes:

  • Route RESTWithNext request failures through handleResponse, aligning its error contract with REST.
  • Remove unreachable non-2xx response handling from RESTWithNext (go-gh already returns an error for non-2xx).
  • Add/extend tests covering error-type parity, gh status 404 tolerance, and attestation retries on 5xx.
Show a summary per file
File Description
api/client.go Converts RESTWithNext errors to CLI api.HTTPError via handleResponse and removes unreachable status handling.
api/client_test.go Adds coverage ensuring RESTWithNext returns api.HTTPError and matches REST’s concrete error type.
pkg/cmd/status/status_test.go Adds a regression test ensuring notifications 404s are tolerated by gh status as intended.
pkg/cmd/attestation/api/client_test.go Adds a regression test ensuring attestation fetch retries fire when RESTWithNext returns a 5xx error.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Low

@williammartin
williammartin changed the base branch from trunk to wm/support-api-host-lift-and-shift July 28, 2026 12:09
@williammartin
williammartin marked this pull request as ready for review July 28, 2026 12:13
@williammartin
williammartin requested review from a team as code owners July 28, 2026 12:13
@williammartin
williammartin requested review from BagToad and removed request for a team July 28, 2026 12:13
@williammartin
williammartin requested a review from tidy-dev July 28, 2026 12:39
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9950859f-e1b7-4129-9ebb-26018d5434bb
@williammartin
williammartin force-pushed the williammartin-fix-restwithnext-error-type branch from d7e2e67 to b46289c Compare July 31, 2026 11:15
@williammartin
williammartin requested review from a team as code owners July 31, 2026 11:15
@williammartin
williammartin changed the base branch from wm/support-api-host-lift-and-shift to trunk July 31, 2026 11:16

@babakks babakks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment thread api/client_test.go
assert.IsType(t, restErr, restWithNextErr)
}

func TestRESTWithNext(t *testing.T) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting that we didn't have a test for this! 🤔

@williammartin
williammartin merged commit e83adbc into trunk Aug 1, 2026
16 checks passed
@williammartin
williammartin deleted the williammartin-fix-restwithnext-error-type branch August 1, 2026 07:52
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Aug 21, 2026
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [cli/cli](https://github.com/cli/cli) | minor | `v2.97.0` → `v2.98.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>cli/cli (cli/cli)</summary>

### [`v2.98.0`](https://github.com/cli/cli/releases/tag/v2.98.0): GitHub CLI 2.98.0

[Compare Source](cli/cli@v2.97.0...v2.98.0)

#### Security

A security vulnerability has been identified, and fixed, that binds the local forwarded port to all available network interfaces by default.

Users of `gh codespace ports forward` are advised to update `gh` to version `v2.98.0` as soon as possible.

For more information see: <GHSA-vfhh-p7hm-pxfh>

#### Support worktrees in `pr checkout`

Users can now checkout a pull request into a git worktree by using the new `--worktree PATH` flag in `gh pr checkout`:

```shell
gh pr checkout 12 --worktree ../wt-feature
```

#### Add semantic search to `search issues`

The `gh search issues` command now supports semantic search for issues. Users can select the search type by passing the `--search-type` flag:

```shell
gh search issues --search-type semantic ...

gh search issues --search-type hybrid ...
```

For more information about semantic search see: ["Improved Search for github issues is now generally available"](https://github.blog/changelog/2026-04-02-improved-search-for-github-issues-is-now-generally-available/).

#### What's Changed

##### ✨ Features

- Add --worktree flag to gh pr checkout by [@&#8203;tidy-dev](https://github.com/tidy-dev) in [#&#8203;13946](cli/cli#13946)
- Set GH\_EXTENSION=1 when gh invokes an extension by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14072](cli/cli#14072)
- Add --search-type flag for semantic and hybrid issue search by [@&#8203;michaeljacholke](https://github.com/michaeljacholke) in [#&#8203;14006](cli/cli#14006)

##### 🐛 Fixes

- Fix `RESTWithNext` error type, repairing `gh status` and attestation retries by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;13988](cli/cli#13988)
- Trim spaces when parsing X-Oauth-Scopes in `gh release create` by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14065](cli/cli#14065)
- Fix project item-add output for non-TTY by [@&#8203;zwick](https://github.com/zwick) in [#&#8203;14056](cli/cli#14056)

##### 📚 Docs & Chores

- Slim down dependabot triage comments by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14019](cli/cli#14019)
- Require explicit MR review ownership by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14028](cli/cli#14028)
- Collapse spam triage into the agentic issue-triage workflow by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14027](cli/cli#14027)
- Run Dependabot triage every hour by [@&#8203;sergiou87](https://github.com/sergiou87) in [#&#8203;14030](cli/cli#14030)
- Route deploy key requests through api.Client by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;13989](cli/cli#13989)
- Route ssh key requests through api.Client by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;13994](cli/cli#13994)
- Route gpg key requests through api.Client by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;13997](cli/cli#13997)
- Route autolink requests through api.Client by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14013](cli/cli#14013)
- Route extension requests through api.Client by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14059](cli/cli#14059)
- Route release creation through api.Client by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14062](cli/cli#14062)
- Tell agents to use the MR template in AGENTS.md by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14074](cli/cli#14074)
- Make Dependabot triage cheaper and more decisive by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14079](cli/cli#14079)
- Route release deletions through api.Client by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14077](cli/cli#14077)
- Give Dependabot triage a real reachability check by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14087](cli/cli#14087)
- Restore automatic spam issue closure by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14088](cli/cli#14088)
- Add a scheduled tech debt burndown skill by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14095](cli/cli#14095)
- Use reflect.Pointer instead of deprecated reflect.Ptr by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14098](cli/cli#14098)
- Clarify what belongs in the MR template's testing section by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14103](cli/cli#14103)
- Rename cli-code-reviewer skill to code-review by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;14116](cli/cli#14116)
- Add aw-actions group to dependabot configuration by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;14123](cli/cli#14123)
- Isolate tests from local machine's auth and git configuration by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;14128](cli/cli#14128)
- Don't ask for feature detection cleanup comments when not needed by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;14139](cli/cli#14139)
- Accept pre-release tags in deployment validation by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;14193](cli/cli#14193)
- ci: add temporary step to verify Linux repo signing keys by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;14202](cli/cli#14202)
- Revert "ci: add temporary step to verify Linux repo signing keys" by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;14203](cli/cli#14203)
- Fix issue triage action compatibility \[skip changelog] by [@&#8203;tidy-dev](https://github.com/tidy-dev) in [#&#8203;14207](cli/cli#14207)

##### :dependabot: Dependencies

- chore(deps): bump github.com/sigstore/sigstore-go from 1.2.2 to 1.3.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14047](cli/cli#14047)
- chore(deps): bump the codeql-actions group across 1 directory with 3 updates by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14049](cli/cli#14049)
- chore(deps): bump google.golang.org/grpc from 1.82.1 to 1.83.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14048](cli/cli#14048)
- chore(deps): bump github.com/google/go-containerregistry from 0.21.7 to 0.21.8 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14066](cli/cli#14066)
- chore(deps): bump actions/attest from 4.2.1 to 4.2.2 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14100](cli/cli#14100)
- chore(deps): bump azure/login from 3.0.0 to 3.0.1 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14101](cli/cli#14101)
- chore(deps): bump the codeql-actions group across 1 directory with 3 updates by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14091](cli/cli#14091)
- chore(deps): bump github/gh-aw-actions/setup-cli from 0.83.4 to 0.85.4 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14068](cli/cli#14068)
- chore(deps): bump github.com/google/go-containerregistry from 0.21.8 to 0.21.9 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14119](cli/cli#14119)
- chore(deps): bump github.com/klauspost/compress from 1.19.1 to 1.19.2 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14120](cli/cli#14120)
- chore(deps): bump the aw-actions group with 2 updates by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14147](cli/cli#14147)
- chore: sign APT repository with both keys by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13271](cli/cli#13271)
- chore(deps): bump github.com/yuin/goldmark from 1.8.4 to 1.8.5 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14029](cli/cli#14029)
- chore(deps): bump actions/attest from 4.2.0 to 4.2.1 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14050](cli/cli#14050)
- Bump golangci-lint in CI to v2.12.2 by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;14102](cli/cli#14102)
- chore(deps): bump the aw-actions group with 2 updates by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14124](cli/cli#14124)
- chore(deps): bump google.golang.org/protobuf from 1.36.11 to 1.36.12 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14140](cli/cli#14140)
- Upgrade gh-aw workflows to v0.85.4 by [@&#8203;tidy-dev](https://github.com/tidy-dev) in [#&#8203;14141](cli/cli#14141)
- Bump Go to 1.26.6 by [@&#8203;github-actions](https://github.com/github-actions)\[bot] in [#&#8203;14143](cli/cli#14143)
- chore: bump go to 1.26.7 by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;14205](cli/cli#14205)
- chore(deps): bump github.com/stretchr/testify from 1.11.1 to 1.12.1 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14204](cli/cli#14204)
- chore(deps): bump the codeql-actions group across 1 directory with 3 updates by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14169](cli/cli#14169)
- chore(deps): bump golang.org/x/crypto from 0.54.0 to 0.55.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14164](cli/cli#14164)
- chore(deps): bump charm.land/lipgloss/v2 from 2.0.5 to 2.0.6 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;14166](cli/cli#14166)
- Bump gh-aw-actions to v0.87.1 and recompile agentic workflows by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;14210](cli/cli#14210)

#### New Contributors

- [@&#8203;sergiou87](https://github.com/sergiou87) made their first contribution in [#&#8203;14030](cli/cli#14030)
- [@&#8203;michaeljacholke](https://github.com/michaeljacholke) made their first contribution in [#&#8203;14006](cli/cli#14006)

**Full Changelog**: <cli/cli@v2.97.0...v2.98.0>

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yODguMCIsInVwZGF0ZWRJblZlciI6IjQzLjI4OC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->
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.

5 participants