fix(js-sdk): preserve code and type from API error responses - #16769
fix(js-sdk): preserve code and type from API error responses#16769robindelaater wants to merge 1 commit into
Conversation
The Store and Admin APIs return errors as `{ code, type, message }`, but
`normalizeResponse` typed the parsed body as `{ message?: string }` and passed
only the message to `FetchError`. The `code` and `type` were dropped before a
consumer could read them.
This left storefronts matching on the English message string to tell one
failure from another, which breaks localization and breaks silently when a
message is reworded. The most common case is `insufficient_inventory` on
add-to-cart or a quantity increase.
`FetchError` now carries `code` and `type`. Both constructor parameters are
optional and appended last, so existing call sites are unaffected.
🦋 Changeset detectedLatest commit: 3b98b19 The changes in this PR will be included in the next version bump. This PR includes changesets to release 83 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for the contribution! We need more information before reviewing this further. The PR adds code and type properties to FetchError so that JS SDK consumers can branch on stable API error identifiers rather than matching on English message strings. The implementation is additive and backward-compatible: two new optional constructor parameters are appended last, normalizeResponse widens its type assertion to include them, and the existing call site passes them through. A unit test is included using an msw handler that returns a realistic Medusa error body, and the test was verified to fail against a revert of client.ts. The changeset is present with the correct patch bump and fix(js-sdk): ... message format. The only issue blocking initial approval is a missing linked issue — the contribution guidelines require one for non-trivial changes, and this fix (new public API surface on FetchError, present since v2.0.0) does not qualify as a trivial one-liner. Triggered by: PR marked as ready for review |
|
Thanks for the contribution! Initial automated review looks good. Re-review. The previous blocking point — a missing linked issue — has been resolved: issue #16770 is now linked via a closing keyword and is open. All other criteria are met. The implementation is additive and fully backward-compatible. Two new optional constructor parameters (code and type) are appended at the end of FetchError's constructor signature, so all existing call sites remain untouched. The normalizeResponse function widens its type assertion to include code and type, and passes them through when present in the error body. Checklist verified: - PR template is fully completed with What, Why, How, and Testing sections. - Linked issue #16770 is real, open, and describes the same problem. - Changeset present at .changeset/js-sdk-preserve-error-code.md, with correct patch bump and fix(js-sdk): … message format. - Unit test added in packages/core/js-sdk/src/tests/client.spec.ts using an msw handler that returns a realistic Medusa error body; all four assertions (status, message, code, type) are covered, and the author verified the test fails against a revert of client.ts. - No security concerns: the code and type values are stored as error properties in a client-side SDK; they do not reach any dangerous sink. - No performance concerns. - No bugs: optional fields default to undefined when absent from the response body, and the JSON parse already has a .catch(() => ({})) guard. - Conventions followed: naming, code style, and no issue/PR references in code comments. Triggered by: PR description updated |
Summary
Fixes #16770
What — What changes are introduced in this PR?
FetchErrornow exposes thecodeandtypethat the API returned alongside the message, andnormalizeResponsereads them from the error body it already parses. Includes a unit test and achangeset.
Why — Why are these changes relevant or necessary?
The Store and Admin APIs return errors as
{ code, type, message }, butnormalizeResponsetypedthe parsed body as
{ message?: string }and passed only the message toFetchError. Thecodeandtypewere dropped before a consumer could read them.This left storefronts matching on the English message string to tell one failure from another, which
breaks localization and breaks silently when a message is reworded. The most common case is
insufficient_inventoryon add-to-cart or a quantity increase, where a shopper should be told theitem is out of stock rather than "something went wrong".
How — How have these changes been implemented?
codeandtype.codeandtypefields toFetchError, passed as two new constructor parameters.Both parameters are optional and appended last, so every existing call site keeps working and no
public signature changes shape. This is additive only, nothing currently reads these fields.
Testing — How have these changes been tested, or how can the reviewer test the feature?
Added
should preserve the code and type returned in the error bodytopackages/core/js-sdk/src/__tests__/client.spec.ts, with an msw handler returning a realistic Medusaerror body.
Verified the test actually covers the bug by reverting only
client.tsand re-running:With the fix in place the full js-sdk suite passes: 4 suites, 33 tests, no regressions.
Also reproduced end to end against a local Medusa backend before and after the change, using a
variant with 5 in stock and requesting 999.
Examples
Before this PR the code is unavailable, so the only way to identify a failure is to match the English
message:
After, a consumer can branch on a stable identifier and map it to its own copy:
Checklist
Please ensure the following before requesting a review:
yarn changesetand follow the promptsAdditional Context
The throw site is unchanged since the v2 launch, I verified the same behaviour in
@medusajs/js-sdk2.0.0, 2.5.0, 2.10.0, 2.15.0 and 2.20.1.
I limited the change to
codeandtypebecause those are the two fields the API returns consistently.If you would rather keep the whole parsed body on the error, that is an easy change happy to follow whichever you prefer.
For context on the impact: the official Next.js starter cannot surface these errors well today
either. Its
medusaErrorhelper (src/lib/util/medusa-error.ts) branches onerror.responseanderror.request, which are Axios properties.FetchErroris fetch-based and has neither, so everyerror falls through to the final branch and a shopper sees:
That is a separate issue in the starter repo, but a
codeonFetchErrorwould give it a clean wayto fix it.