Skip to content

Commit b6d3713

Browse files
committed
refactor(github): tighten error messages + remove quiet option
Two cleanups on top of the v5.26.0 work: * `quiet` option removed from `getLatestRelease` and `getReleaseAssetUrl` type signatures. The helpers are silent by design now (errors throw, success returns); there's nothing for `quiet` to suppress. Per the repo's "no backward compat" policy, the option is deleted rather than accepted-but-ignored. Internal callers (`socket-btm.ts`, `downloadGitHubRelease`, `downloadReleaseAsset`) updated to drop the now-unused option arg. CHANGELOG entry moved to `### Removed`. * All error messages I touched in this release reviewed against the CLAUDE.md "Errors are a UX surface" guidance. Library-API throws should be terse, stable, and include where (owner/repo). Fixes: - lowercase `failed to resolve ref` -> capitalized to match siblings - `Failed to fetch releases:` -> `Failed to fetch ${owner}/${repo} releases:` - `Failed to parse GitHub releases response from <hardcoded URL>` -> `Failed to parse ${owner}/${repo} releases response` - `GraphQL error: <messages>` -> `GraphQL securityAdvisory(...) returned errors: <messages>` / `GraphQL repository.releases(...) returned errors: ...` / `GraphQL repository.release(...) returned errors: ...` (which query failed is now visible) - `GHSA X not found via GraphQL` -> `GHSA X not found` (transport detail leaked; users don't care which transport) - `GraphQL fallback for X failed: ...` -> `Failed to fetch ${owner}/${repo} release X (GraphQL): ...` - `Release X not found in Y: GraphQL fallback found no release with that tag` -> `Release X not found in Y` (redundant tail removed) - `Failed to parse GitHub release response for tag X` -> `Failed to parse ${owner}/${repo} release X response` - `Failed to fetch release X: 503` -> `Failed to fetch ${owner}/${repo} release X: 503` - `Release X has no assets` -> `Release X has no assets in ${owner}/${repo}` - `Failed to parse GitHub GraphQL release response for X` -> `Failed to parse ${owner}/${repo} release X response (GraphQL)` Test assertions updated to the new wording. 154/154 still pass. tsgo + lint clean.
1 parent 60f8c7a commit b6d3713

6 files changed

Lines changed: 83 additions & 137 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1616

1717
- `getLatestRelease`, `getReleaseAssetUrl`, `fetchRefShaViaGraphQL` (internal), `fetchReleaseAssetsViaGraphQL` (internal) — return `undefined` (was: `null`) when no result is found. Matches the `__proto__: null` only / `undefined` convention used elsewhere in the package. Callers using `=== null` will need to switch to `=== undefined` or a falsy check; callers using `if (!result)` are unaffected
1818
- `fetchGhsaDetails` GraphQL fallback path normalizes severity to lowercase (`"MODERATE"``"moderate"`) to match the REST endpoint's wire shape. Callers comparing against a single canonical case no longer have to handle both
19-
- `getLatestRelease` and `getReleaseAssetUrl` no longer log to `logger.info` / `logger.warn` on success, retry, or fallback. The helpers are silent by design now: errors throw, success returns. The `quiet` option is still accepted for backward compat but ignored
19+
- `getLatestRelease` and `getReleaseAssetUrl` are silent by design now: errors throw, success returns. No more `logger.info` / `logger.warn` calls on success, retry, or fallback
20+
21+
### Removed
22+
23+
- `getLatestRelease({ quiet })` and `getReleaseAssetUrl({ quiet })` — the `quiet` option is gone from the type signatures. Passing it is now a TypeScript error. There's nothing left to suppress (the helpers don't log anymore — see "Changed"). Callers that were passing `{ quiet: true }` as their only option should drop the options arg entirely
2024

2125
### Fixed
2226

src/github.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ async function fetchRefSha(
417417
// the absence). Surface the cleaner "ref not found" message.
418418
}
419419
throw new ErrorCtor(
420-
`failed to resolve ref "${ref}" for ${owner}/${repo}: ${errorMessage(e3)}`,
420+
`Failed to resolve ref "${ref}" for ${owner}/${repo}: ${errorMessage(e3)}`,
421421
)
422422
}
423423
}
@@ -923,12 +923,12 @@ async function fetchGhsaDetailsViaGraphQL(
923923
}
924924
if (parsed.errors?.length) {
925925
throw new ErrorCtor(
926-
`GraphQL error: ${parsed.errors.map(e => e.message).join('; ')}`,
926+
`GraphQL securityAdvisory(${ghsaId}) returned errors: ${parsed.errors.map(e => e.message).join('; ')}`,
927927
)
928928
}
929929
const adv = parsed.data?.securityAdvisory
930930
if (!adv) {
931-
throw new ErrorCtor(`GHSA ${ghsaId} not found via GraphQL`)
931+
throw new ErrorCtor(`GHSA ${ghsaId} not found`)
932932
}
933933
return {
934934
ghsaId: adv.ghsaId,

src/releases/github.ts

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -384,11 +384,7 @@ export async function downloadGitHubRelease(
384384
if (explicitTag) {
385385
tag = explicitTag
386386
} else if (toolPrefix) {
387-
const latestTag = await getLatestRelease(
388-
toolPrefix,
389-
{ owner, repo },
390-
{ quiet },
391-
)
387+
const latestTag = await getLatestRelease(toolPrefix, { owner, repo })
392388
if (!latestTag) {
393389
throw new Error(`No ${toolPrefix} release found in ${owner}/${repo}`)
394390
}
@@ -495,12 +491,10 @@ export async function downloadReleaseAsset(
495491
const { quiet = false } = options
496492

497493
// Get the browser_download_url for the asset.
498-
const downloadUrl = await getReleaseAssetUrl(
499-
tag,
500-
assetPattern,
501-
{ owner, repo },
502-
{ quiet },
503-
)
494+
const downloadUrl = await getReleaseAssetUrl(tag, assetPattern, {
495+
owner,
496+
repo,
497+
})
504498

505499
if (!downloadUrl) {
506500
const patternDesc =
@@ -593,7 +587,9 @@ async function fetchReleasesViaRest(
593587
{ headers: getAuthHeaders() },
594588
)
595589
if (!response.ok) {
596-
throw new ErrorCtor(`Failed to fetch releases: ${response.status}`)
590+
throw new ErrorCtor(
591+
`Failed to fetch ${owner}/${repo} releases: ${response.status}`,
592+
)
597593
}
598594
const text = response.body.toString('utf8')
599595
if (text.length === 0) {
@@ -607,10 +603,9 @@ async function fetchReleasesViaRest(
607603
try {
608604
parsed = JSONParse(text)
609605
} catch (cause) {
610-
throw new ErrorCtor(
611-
`Failed to parse GitHub releases response from https://api.github.com/repos/${owner}/${repo}/releases`,
612-
{ cause },
613-
)
606+
throw new ErrorCtor(`Failed to parse ${owner}/${repo} releases response`, {
607+
cause,
608+
})
614609
}
615610
return ArrayIsArray(parsed) ? (parsed as ReleaseRow[]) : []
616611
}
@@ -667,7 +662,7 @@ async function fetchReleasesViaGraphQL(
667662
})
668663
if (!response.ok) {
669664
throw new ErrorCtor(
670-
`Failed to fetch releases via GraphQL: ${response.status}`,
665+
`Failed to fetch ${owner}/${repo} releases (GraphQL): ${response.status}`,
671666
)
672667
}
673668
let parsed: {
@@ -694,7 +689,7 @@ async function fetchReleasesViaGraphQL(
694689
}
695690
if (parsed.errors?.length) {
696691
throw new ErrorCtor(
697-
`GraphQL error: ${parsed.errors.map(e => e.message).join('; ')}`,
692+
`GraphQL repository.releases(${owner}/${repo}) returned errors: ${parsed.errors.map(e => e.message).join('; ')}`,
698693
)
699694
}
700695
return (parsed.data?.repository?.releases?.nodes ?? []).map(n => ({
@@ -757,12 +752,12 @@ async function fetchReleaseAssetsViaGraphQL(
757752
})
758753
if (!response.ok) {
759754
throw new ErrorCtor(
760-
`GraphQL fallback for ${tag} failed: ${response.status} ${response.statusText}`,
755+
`Failed to fetch ${owner}/${repo} release ${tag} (GraphQL): ${response.status} ${response.statusText}`,
761756
)
762757
}
763758
if (response.body.byteLength === 0) {
764759
throw new ErrorCtor(
765-
`GraphQL fallback for ${tag} also returned empty body — both REST and GraphQL backends are degraded`,
760+
`Failed to fetch ${owner}/${repo} release ${tag}: GraphQL returned empty body`,
766761
)
767762
}
768763
let parsed: {
@@ -782,13 +777,13 @@ async function fetchReleaseAssetsViaGraphQL(
782777
parsed = JSONParse(response.body.toString('utf8'))
783778
} catch (cause) {
784779
throw new ErrorCtor(
785-
`Failed to parse GitHub GraphQL release response for ${tag}`,
780+
`Failed to parse ${owner}/${repo} release ${tag} response (GraphQL)`,
786781
{ cause },
787782
)
788783
}
789784
if (parsed.errors?.length) {
790785
throw new ErrorCtor(
791-
`GraphQL error fetching release ${tag}: ${parsed.errors.map(e => e.message).join('; ')}`,
786+
`GraphQL repository.release(${owner}/${repo}, ${tag}) returned errors: ${parsed.errors.map(e => e.message).join('; ')}`,
792787
)
793788
}
794789
const release = parsed.data?.repository?.release
@@ -813,7 +808,6 @@ async function fetchReleaseAssetsViaGraphQL(
813808
* @param options - Additional options
814809
* @param options.assetPattern - Optional pattern to filter releases by matching asset
815810
* @param options.nothrow - If true, return undefined instead of throwing when both REST and GraphQL backends are degraded. Default: false.
816-
* @param options.quiet - Accepted for backward compat; no longer consumed.
817811
* @returns Latest release tag or undefined if not found
818812
* @throws {Error} If both REST and GraphQL backends are degraded and nothrow is false.
819813
*
@@ -831,11 +825,12 @@ export async function getLatestRelease(
831825
options: {
832826
assetPattern?: AssetPattern
833827
nothrow?: boolean
834-
quiet?: boolean
835828
} = {},
836829
): Promise<string | undefined> {
837-
// `quiet` is accepted for backward compat but no longer consumed —
838-
// the helper is silent by design now (errors throw, success returns).
830+
// The `quiet` option from previous releases is no longer accepted.
831+
// The helper is silent by design now (errors throw, success
832+
// returns) so there's nothing for the caller to suppress. Type
833+
// enforces this — passing `{ quiet: true }` is a TS error.
839834
const { assetPattern, nothrow = false } = options
840835
const { owner, repo } = repoConfig
841836

@@ -943,7 +938,6 @@ export async function getLatestRelease(
943938
* @param repoConfig - Repository configuration (owner/repo)
944939
* @param options - Additional options
945940
* @param options.nothrow - If true, return undefined instead of throwing when both REST and GraphQL backends are degraded. Default: false.
946-
* @param options.quiet - Accepted for backward compat; no longer consumed.
947941
* @returns Browser download URL for the asset, or undefined when not found.
948942
* @throws {Error} If both REST and GraphQL backends are degraded and nothrow is false.
949943
*
@@ -959,8 +953,11 @@ export async function getReleaseAssetUrl(
959953
tag: string,
960954
assetPattern: string | AssetPattern,
961955
repoConfig: RepoConfig,
962-
options: { nothrow?: boolean; quiet?: boolean } = {},
956+
options: { nothrow?: boolean } = {},
963957
): Promise<string | undefined> {
958+
// The `quiet` option from previous releases is no longer accepted.
959+
// The helper is silent by design now (errors throw, success
960+
// returns). Type enforces this — passing `{ quiet: true }` is a TS error.
964961
const { nothrow = false } = options
965962
const { owner, repo } = repoConfig
966963

@@ -982,7 +979,9 @@ export async function getReleaseAssetUrl(
982979
)
983980

984981
if (!response.ok) {
985-
throw new Error(`Failed to fetch release ${tag}: ${response.status}`)
982+
throw new ErrorCtor(
983+
`Failed to fetch ${owner}/${repo} release ${tag}: ${response.status}`,
984+
)
986985
}
987986

988987
// -------------------------------------------------------
@@ -1034,9 +1033,7 @@ export async function getReleaseAssetUrl(
10341033
if (nothrow) {
10351034
return undefined
10361035
}
1037-
throw new ErrorCtor(
1038-
`Release ${tag} not found in ${owner}/${repo}: GraphQL fallback found no release with that tag`,
1039-
)
1036+
throw new ErrorCtor(`Release ${tag} not found in ${owner}/${repo}`)
10401037
}
10411038
assets = fallbackAssets
10421039
} else {
@@ -1047,13 +1044,15 @@ export async function getReleaseAssetUrl(
10471044
release = JSONParse(response.body.toString('utf8'))
10481045
} catch (cause) {
10491046
throw new ErrorCtor(
1050-
`Failed to parse GitHub release response for tag ${tag}`,
1047+
`Failed to parse ${owner}/${repo} release ${tag} response`,
10511048
{ cause },
10521049
)
10531050
}
10541051

10551052
if (!ArrayIsArray(release.assets)) {
1056-
throw new ErrorCtor(`Release ${tag} has no assets`)
1053+
throw new ErrorCtor(
1054+
`Release ${tag} has no assets in ${owner}/${repo}`,
1055+
)
10571056
}
10581057
assets = release.assets
10591058
}

src/releases/socket-btm.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,6 @@ export async function downloadSocketBtmRelease(
233233
resolvedTag =
234234
(await getLatestRelease(toolPrefix, SOCKET_BTM_REPO, {
235235
assetPattern: asset,
236-
quiet,
237236
})) ?? undefined
238237

239238
if (!resolvedTag) {
@@ -244,9 +243,6 @@ export async function downloadSocketBtmRelease(
244243
resolvedTag,
245244
asset,
246245
SOCKET_BTM_REPO,
247-
{
248-
quiet,
249-
},
250246
)
251247

252248
if (!assetUrl) {

test/unit/github.test.mts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,7 @@ describe.sequential('github', () => {
703703

704704
await expect(
705705
resolveRefToSha('owner', 'repo', 'nonexistent'),
706-
).rejects.toThrow('failed to resolve ref')
706+
).rejects.toThrow('Failed to resolve ref')
707707
})
708708

709709
it('should fall back to GraphQL when REST returns 200 + empty body', async () => {
@@ -827,14 +827,14 @@ describe.sequential('github', () => {
827827

828828
await expect(
829829
resolveRefToSha('owner', 'repo', 'genuinely-missing-ref'),
830-
).rejects.toThrow('failed to resolve ref')
830+
).rejects.toThrow('Failed to resolve ref')
831831
})
832832

833833
it('should re-throw original REST error when GraphQL fallback also fails', async () => {
834834
// Both transports degraded: REST hits empty bodies all the way
835835
// through the cascade, GraphQL returns a non-OK status. The
836836
// helper should swallow the GraphQL transport error and
837-
// surface the REST cascade's 'failed to resolve ref' message
837+
// surface the REST cascade's 'Failed to resolve ref' message
838838
// — that's more actionable for the user than a confusing
839839
// GraphQL-side error caused by the same incident.
840840
nock('https://api.github.com')
@@ -849,7 +849,7 @@ describe.sequential('github', () => {
849849

850850
await expect(
851851
resolveRefToSha('owner', 'repo', 'double-failure-ref'),
852-
).rejects.toThrow('failed to resolve ref')
852+
).rejects.toThrow('Failed to resolve ref')
853853
})
854854

855855
it('should return undefined from GraphQL when ref not found anywhere', async () => {
@@ -876,7 +876,7 @@ describe.sequential('github', () => {
876876

877877
await expect(
878878
resolveRefToSha('owner', 'repo', 'incident-but-real-404'),
879-
).rejects.toThrow('failed to resolve ref')
879+
).rejects.toThrow('Failed to resolve ref')
880880
})
881881

882882
it('should forward auth token to GraphQL fallback', async () => {

0 commit comments

Comments
 (0)