Background
CodeRabbit flagged this on #41 (review comment) for the GPA surface, but the issue is class-wide: every public method on GhinClient returns Promise<T> and throws on error, which contradicts the repo's stated convention:
neverthrow Result types for error handling — functions return Result<T, E> instead of throwing
— CLAUDE.md
The underlying RequestClient already returns Result<T, E>; GhinClient unwraps it (throwing on isErr()) before handing values back to callers, discarding the typed-error guarantee at the public boundary.
Scope
All 7 public surfaces on GhinClient (~28 methods) in src/client/ghin/index.ts:
courses — 5 methods (getCountries, getDetails, search, getTeeSetRating, getTeeSetRatingsForScorePosting)
facilities — 1 method (search)
golfers — 4 methods (getOne, getScores, search, globalSearch)
gpa — 4 methods (getAccesses, requestAccess, updateStatus, revokeAccess)
handicaps — 4 methods (getOne, getCoursePlayerHandicaps, getCourseHandicaps, getPlayingHandicaps)
scores — 3 methods (postHoleByHole, postAdjusted, post18h9and9)
webhooks — 7 methods (get, patch, delete, test, list, resend, ensureRegistered)
Each method signature changes from Promise<T> → Promise<Result<T, GhinError>> (or a more specific error union per surface).
Why all at once
Migrating only one surface (e.g. GPA) would leave an inconsistent public API where some methods throw and others return Results — strictly worse than the current uniform-but-wrong state. The change is mechanical but breaking, so it should land in a single release.
Out of scope
- Internal helpers and
RequestClient already use Result; no changes needed there.
- Error-class hierarchy under
src/errors/ is fine as-is; this is purely a wrapping change.
Acceptance criteria
Background
CodeRabbit flagged this on #41 (review comment) for the GPA surface, but the issue is class-wide: every public method on
GhinClientreturnsPromise<T>and throws on error, which contradicts the repo's stated convention:The underlying
RequestClientalready returnsResult<T, E>;GhinClientunwraps it (throwing onisErr()) before handing values back to callers, discarding the typed-error guarantee at the public boundary.Scope
All 7 public surfaces on
GhinClient(~28 methods) insrc/client/ghin/index.ts:courses— 5 methods (getCountries,getDetails,search,getTeeSetRating,getTeeSetRatingsForScorePosting)facilities— 1 method (search)golfers— 4 methods (getOne,getScores,search,globalSearch)gpa— 4 methods (getAccesses,requestAccess,updateStatus,revokeAccess)handicaps— 4 methods (getOne,getCoursePlayerHandicaps,getCourseHandicaps,getPlayingHandicaps)scores— 3 methods (postHoleByHole,postAdjusted,post18h9and9)webhooks— 7 methods (get,patch,delete,test,list,resend,ensureRegistered)Each method signature changes from
Promise<T>→Promise<Result<T, GhinError>>(or a more specific error union per surface).Why all at once
Migrating only one surface (e.g. GPA) would leave an inconsistent public API where some methods throw and others return Results — strictly worse than the current uniform-but-wrong state. The change is mechanical but breaking, so it should land in a single release.
Out of scope
RequestClientalready use Result; no changes needed there.src/errors/is fine as-is; this is purely a wrapping change.Acceptance criteria
GhinClientreturnPromise<Result<T, E>>result.isOk()/result.isErr()instead of.rejects.toThrow(...)src/playground/updated to the new shape