Skip to content

Fix RequestManager de-duplication key to be stable across object key order#5781

Closed
Chaitu7032 wants to merge 1 commit intosugarlabs:masterfrom
Chaitu7032:RequestMan
Closed

Fix RequestManager de-duplication key to be stable across object key order#5781
Chaitu7032 wants to merge 1 commit intosugarlabs:masterfrom
Chaitu7032:RequestMan

Conversation

@Chaitu7032
Copy link
Contributor

Summary

This PR fixes a subtle de-duplication issue in RequestManager where two logically identical request payloads could generate different request keys due to object property insertion order.

Impact:

  • Prevents duplicate in-flight API calls
  • Reduces unnecessary API load and rate-limit risk
  • Makes cachedResponses stats more accurate

The change is intentionally small, isolated, and low risk.

Problem (Before)

generateRequestKey() was building request keys using:

JSON.stringify(params) ...

However, JSON.stringify() preserves object insertion order.
This means the same fields assembled via different spreads or merges could stringify differently.

Result:

  • Two equivalent requests → different keys
  • pendingRequests failed to detect duplicates
  • Identical requests could run concurrently

This bug is silent: no errors, just unnecessary duplicate requests.

Fix (What Changed)

A small helper, stableStringify(), was added to normalize request parameters before stringification:

  • Recursively sorts keys for plain objects (constructor === Object)
  • Normalizes arrays recursively
  • Delegates actual serialization and error handling to native JSON.stringify()

generateRequestKey() now uses:

stableStringify(params) ... instead of raw JSON.stringify(params).

Files Changed

planet/js/RequestManager.js
planet/js/__tests__/RequestManager.test.js .

How to Verify (Before vs After) ..

Before (root cause: order-dependent stringify)

image

Expected output: equal? false

After (fixed behavior: stable request keys)

image Expected output: `equal? true`

tested screenshots

before

Screenshot 2026-02-18 171750

after fix

Screenshot 2026-02-18 171735

tests

Added regression coverage to ensure:

  • Request keys are stable across different object key orders
  • Nested objects are handled deterministically
  • throttledRequest() correctly de-dupes concurrent calls even when payload key order differs

Run: npm test -- planet/js/__tests__/RequestManager.test.js

Screenshot 2026-02-18 171934

Risk / Compatibility

  • Low risk: change is isolated to request key generation
  • No impact on request execution, retries, throttling, or API semantics
  • Behavior change is limited to improved de-duplication correctness

Note: npm test may print an unrelated coverage parse warning from another file in the repo; the RequestManager test suite passes and validates this change.

Why this matters

This fix removes a subtle but real source of duplicate API traffic while keeping the implementation minimal, predictable, and easy to reason about.

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@Chaitu7032 Chaitu7032 closed this Feb 18, 2026
@Chaitu7032 Chaitu7032 deleted the RequestMan branch February 18, 2026 12:36
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.

1 participant