fix(trading): derive X-EBAY-API-SITEID from EBAY_MARKETPLACE_ID#141
Open
teetlaja wants to merge 1 commit into
Open
fix(trading): derive X-EBAY-API-SITEID from EBAY_MARKETPLACE_ID#141teetlaja wants to merge 1 commit into
teetlaja wants to merge 1 commit into
Conversation
The Trading API client hardcoded site ID 0 (eBay US), so every Trading call ignored EBAY_MARKETPLACE_ID. Reads still worked, but any listing write on a non-US marketplace failed at eBay with "Invalid auction currency", because the currency is validated against the site: an EUR listing cannot be created on site 0. That made create/revise listing unusable on every marketplace except the US, with no way to configure around it. Site ID is now derived from EBAY_MARKETPLACE_ID via the fixed eBay mapping (EBAY_DE -> 77, EBAY_GB -> 3, ...), so the marketplace setting drives REST and Trading traffic alike. EBAY_SITE_ID overrides it for sites the map does not cover, or when REST and Trading must target different sites. Unknown marketplaces keep falling back to 0, so US setups are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🤖 CodeAnt AI — Review Status
Updated in place by CodeAnt AI · last 5 reviews |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
📝 WalkthroughWalkthroughTrading API site IDs are now derived from ChangesTrading API site configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
|
||
| const buildTradingHeaders = (callName: string): Record<string, string> => ({ | ||
| 'X-EBAY-API-SITEID': SITE_ID, | ||
| 'X-EBAY-API-SITEID': getTradingSiteId(), |
There was a problem hiding this comment.
Suggestion: The Trading header is derived from process-wide environment instead of the active client configuration, so REST calls can target one marketplace while Trading calls use a different site when the API is instantiated with a custom config object. Derive the site ID from the same runtime config used by the EbayApiClient to keep both protocols aligned per client instance. [api mismatch]
Severity Level: Major ⚠️
❌ Trading createListing uses site from global env.
❌ Trading reviseListing routed to wrong marketplace site.
⚠️ Embedded consumers using custom EbayConfig see protocol drift.
⚠️ Debugging marketplace mismatch across REST/Trading is harder.Steps of Reproduction ✅
1. An embedded consumer constructs a custom eBay configuration object (type `EbayConfig`
from `src/types/ebay.js`) where `marketplaceId` is set to a non-US marketplace such as
`'EBAY_DE'`, and passes it into the public facade `new EbaySellerApi(config)` defined in
`src/api/index.ts:31-58`.
2. The `EbaySellerApi` constructor at `src/api/index.ts:58-84` builds a shared REST client
via `new EbayApiClient(config)` and then instantiates a Trading client with `const
tradingClient = new TradingApiClient(this.client);` (`TradingApiClient` constructor in
`src/api/clientTrading.ts:234-239`).
3. The REST client uses the per-instance configuration to set marketplace headers:
`EbayApiClient.getDefaultHeaders()` in `src/api/client.ts:89-105` adds
`headers['X-EBAY-C-MARKETPLACE-ID'] = this.config.marketplaceId;`, so REST requests
correctly target the custom marketplace from the `config` object rather than
`process.env`.
4. A Trading write is performed via a high-level method such as
`tradingApi.createListing()` in `src/api/trading/trading.ts:148-158`, which calls
`tradingClient.execute('AddFixedPriceItem', {...})`. Inside `TradingApiClient.execute`
(`src/api/clientTrading.ts:35-71`), request headers are built with
`buildTradingHeaders(callName)` at `src/api/clientTrading.ts:51-56`, which sets
`'X-EBAY-API-SITEID': getTradingSiteId()`. `getTradingSiteId()` in
`src/config/environment.ts:364-371` ignores the client configuration and instead derives
the site purely from process-wide environment (`EBAY_SITE_ID` / `EBAY_MARKETPLACE_ID`), so
if those environment variables do not match the custom `config.marketplaceId`, REST and
Trading traffic are routed to different sites, and Trading writes for the configured
marketplace can fail at eBay due to site/currency mismatch.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/api/clientTrading.ts
**Line:** 52:52
**Comment:**
*Api Mismatch: The Trading header is derived from process-wide environment instead of the active client configuration, so REST calls can target one marketplace while Trading calls use a different site when the API is instantiated with a custom config object. Derive the site ID from the same runtime config used by the `EbayApiClient` to keep both protocols aligned per client instance.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+365
to
+368
| const override = (process.env.EBAY_SITE_ID ?? '').trim(); | ||
| if (override !== '') { | ||
| return override; | ||
| } |
There was a problem hiding this comment.
Suggestion: The override is returned as-is for any non-empty value, but Trading expects a numeric site ID; a typo like EBAY_SITE_ID=EBAY_DE will produce invalid headers and fail every Trading write at runtime. Validate that the override is numeric (and ideally within known site IDs) before using it, otherwise reject or fall back safely. [api mismatch]
Severity Level: Major ⚠️
❌ Trading createListing fails when EBAY_SITE_ID is mistyped.
❌ Trading reviseListing fails due to invalid numeric site header.
⚠️ Misconfiguration leads to hard-to-diagnose Trading write errors.Steps of Reproduction ✅
1. Configure the environment for a non-US marketplace (for example,
`process.env.EBAY_MARKETPLACE_ID = 'EBAY_DE'`) and set an invalid Trading override such as
`process.env.EBAY_SITE_ID = 'EBAY_DE'` instead of the numeric site ID. The override
handling in `getTradingSiteId()` at `src/config/environment.ts:364-371` reads `const
override = (process.env.EBAY_SITE_ID ?? '').trim();` and returns it verbatim whenever it
is non-empty.
2. When any Trading operation is invoked via the public API — e.g.,
`tradingApi.createListing()` in `src/api/trading/trading.ts:148-158` — the call flows into
`TradingApiClient.execute()` at `src/api/clientTrading.ts:35-71`. This method constructs
headers with `const headers = buildTradingHeaders(callName);` where
`buildTradingHeaders()` (`src/api/clientTrading.ts:51-56`) sets `'X-EBAY-API-SITEID':
getTradingSiteId()`.
3. With `EBAY_SITE_ID` misconfigured to `'EBAY_DE'`, `getTradingSiteId()` returns this
non-numeric string, so `buildTradingHeaders()` sends `X-EBAY-API-SITEID: EBAY_DE` on every
Trading request. The Trading API expects a numeric site ID as documented in the comments
at `src/config/environment.ts:47-52`, so eBay will reject or mishandle these requests,
causing the HTTP call in `postTradingXml()` (`src/api/clientTrading.ts:113-130`) to fail
or return error responses.
4. The failure is surfaced to callers through the error-handling chain:
`createTradingApiError()` at `src/api/clientTrading.ts:73-87` wraps the Trading failure,
and `validateTradingAck()` at `src/api/clientTrading.ts:197-223` returns an `Effect.fail`
when `Ack` indicates failure. Because there is no validation of `EBAY_SITE_ID` being
numeric in `getTradingSiteId()` or in `validateEnvironmentConfig()`
(`src/config/environment.ts:57-107`), any typo or non-numeric override value consistently
breaks all Trading writes at runtime until the environment is corrected.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/config/environment.ts
**Line:** 365:368
**Comment:**
*Api Mismatch: The override is returned as-is for any non-empty value, but Trading expects a numeric site ID; a typo like `EBAY_SITE_ID=EBAY_DE` will produce invalid headers and fail every Trading write at runtime. Validate that the override is numeric (and ideally within known site IDs) before using it, otherwise reject or fall back safely.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 4/5
- In
src/config/environment.ts,EBAY_SITE_IDcurrently accepts nonnumeric values, so a simple config typo can propagate into every Trading API call and cause hard-to-diagnose request failures in production. ValidateEBAY_SITE_IDas numeric at startup and fail fast with a clear configuration error before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/config/environment.ts">
<violation number="1" location="src/config/environment.ts:367">
P2: A nonnumeric `EBAY_SITE_ID` is accepted and attached to every Trading request, causing opaque API failures from a simple configuration typo. Validate the override as a numeric site ID and surface a configuration error before sending calls.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| export const getTradingSiteId = (): string => { | ||
| const override = (process.env.EBAY_SITE_ID ?? '').trim(); | ||
| if (override !== '') { | ||
| return override; |
There was a problem hiding this comment.
P2: A nonnumeric EBAY_SITE_ID is accepted and attached to every Trading request, causing opaque API failures from a simple configuration typo. Validate the override as a numeric site ID and surface a configuration error before sending calls.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/config/environment.ts, line 367:
<comment>A nonnumeric `EBAY_SITE_ID` is accepted and attached to every Trading request, causing opaque API failures from a simple configuration typo. Validate the override as a numeric site ID and surface a configuration error before sending calls.</comment>
<file context>
@@ -311,6 +311,65 @@ export const getEbayConfig = (): EbayConfig => {
+export const getTradingSiteId = (): string => {
+ const override = (process.env.EBAY_SITE_ID ?? '').trim();
+ if (override !== '') {
+ return override;
+ }
+ const marketplaceId = (process.env.EBAY_MARKETPLACE_ID ?? '').trim() || 'EBAY_US';
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Problem
src/api/clientTrading.tshardcoded the Trading API site:EBAY_MARKETPLACE_IDnever reached the Trading API — it is read ingetEbayConfig()and only becomes theX-EBAY-C-MARKETPLACE-IDheader on REST calls.clientTrading.tsimports justgetBaseUrlfrom the config module.Reads still work (own-listing reads are not site-scoped), so the misconfiguration is invisible until the first write. Creating a listing on a non-US marketplace then fails at eBay:
The currency is validated against the site, so an EUR listing sent to site
0is rejected. In practiceebay_create_listing/ebay_revise_listingare unusable on every marketplace except the US, with no environment variable to work around it. Hit this on a live eBay.de seller account withEBAY_MARKETPLACE_ID=EBAY_DEset.Fix
Derive the site ID from
EBAY_MARKETPLACE_IDusing eBay's fixed mapping (EBAY_DE→77,EBAY_GB→3, …), so one setting drives REST and Trading traffic alike.EBAY_SITE_IDis added as an override for sites the map does not cover, or when REST and Trading must intentionally differ.Unknown or unset marketplaces still resolve to
0, so existing US setups behave exactly as before.Changes
src/config/environment.ts—TRADING_SITE_IDSmap + exportedgetTradingSiteId()src/api/clientTrading.ts— header usesgetTradingSiteId()instead of the constanttests/unit/config/environment.test.ts— 5 tests: marketplace mapping, unset default, unknown-marketplace fallback, override, blank-override ignoredREADME.md+docs/auth/CONFIGURATION.md— documentEBAY_SITE_IDand the REST/Trading header splitVerification
npx vitest run→ 1051 passed (59 files)npx tsc --noEmit→ cleanAddFixedPriceItempayload that returnedInvalid auction currencyon site0is accepted withEBAY_MARKETPLACE_ID=EBAY_DE.Note:
biome check src/api/clientTrading.tspanics with an internal error, but it does so on unmodifiedmaintoo — pre-existing, unrelated to this change.Summary by cubic
Derives Trading’s
X-EBAY-API-SITEIDfromEBAY_MARKETPLACE_IDso listing writes work on non-US sites and no longer fail with “Invalid auction currency.” AddsEBAY_SITE_IDas an optional override; US behavior stays the default.getTradingSiteId()mapping and used it insrc/api/clientTrading.ts.0(US); blank override ignored.EBAY_SITE_IDenv var to override the mapping when needed.Written for commit 8ad9363. Summary will update on new commits.
CodeAnt-AI Description
Use the correct Trading API site for the chosen marketplace
What Changed
EBAY_MARKETPLACE_IDinstead of always using the US siteEBAY_SITE_IDcan override the mapped site when a marketplace is not covered or REST and Trading need different sitesImpact
✅ Fewer listing failures on non-US eBay accounts✅ Clearer marketplace-to-site behavior✅ Lower risk of "Invalid auction currency" errors💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
New Features
EBAY_SITE_ID.Bug Fixes
Documentation
EBAY_SITE_ID, including defaults, overrides, and API-specific behavior.