[Shopify] Tolerate legacy bulk-operation request data missing unitCost key#8386
Open
onbuyuka wants to merge 1 commit into
Open
[Shopify] Tolerate legacy bulk-operation request data missing unitCost key#8386onbuyuka wants to merge 1 commit into
onbuyuka wants to merge 1 commit into
Conversation
…t key Bulk variant price updates persist a JSON "Request Data" blob on Shpfy Bulk Operation so the rollback path can restore the previous Price / Compare-at-Price / Updated At / Unit Cost values when Shopify's async callback reports failure. PR #6155 added a new 'unitCost' key to that blob and a matching reader on the rollback path, but the reader uses JsonObject.GetDecimal which throws if the key is missing. Any bulk operation queued before the upgrade (no 'unitCost' in the blob) and revertd after the upgrade crashes with "There are no properties with the key 'unitCost' on the JSON object". Guard the 'unitCost' read with JsonObject.Contains in both rollback sites (ShpfyBulkUpdateProductPrice.RevertRequests and ShpfyProductExport.RevertVariantChanges). When absent, Unit Cost is left untouched, matching the pre-PR-#6155 behaviour. Adds a regression test simulating a legacy request-data blob without the unitCost key. Fixes AB#637250 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
darjoo
approved these changes
Jun 1, 2026
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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
2 participants
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.
Summary
Bulk variant price updates persist a JSON
Request Datablob onShpfy Bulk Operationso the rollback path can restore the previous Price / Compare-at-Price / Updated At / Unit Cost values when Shopify's async callback reports failure.PR #6155 added a new
unitCostkey to that blob and a matching reader on the rollback path, but the reader usesJsonObject.GetDecimalwhich throws if the key is missing. Any bulk operation queued before the upgrade (nounitCostin the blob) and reverted after the upgrade crashes with:The same stale rows can keep retrying, so the error surfaces repeatedly even though the upgrade happened once.
The Shopify API payload itself is unaffected — it still correctly sends
inventoryItem.cost. The crash is purely on the BC-side rollback reader.Changes
Production code
ShpfyBulkUpdateProductPrice.RevertRequestsandShpfyProductExport.RevertVariantChanges: guard theunitCostread withJsonObject.Contains('unitCost'). If absent (legacy blob), Unit Cost is left untouched, matching the pre-PR-[Shopify] Update cost when syncing prices #6155 behaviour.Test code
TestBulkOperationRevertFailedWithLegacyRequestDataMissingUnitCost: new regression test that builds a request-data blob without theunitCostkey (via aGenerateLegacyRequestDataWithoutUnitCosthelper) and asserts that the rollback completes without throwing, reverts Price / Compare-at-Price correctly, and leaves Unit Cost untouched.Why only
unitCostis gated, not every fieldThe other keys (
id,price,compareAtPrice,updatedAt) have been written byShpfyVariantAPI.UpdateProductPricesince the connector was first migrated to BCApps — there is no version that ever persisted aRequest Datablob without them. A missingprice/compareAtPrice/updatedAt/idis therefore not a legitimate "legacy blob" scenario; it would be a real upstream bug (regressed writer, corrupted blob, forgottenJRequest.Add).GetDecimal's throw is the correct behaviour there — surfacing the bug loudly is better than silently skipping a rollback (which would leave variants showing the wrong price in BC, or no-op the revert with zero diagnostic).unitCostis the one and only key that was added later, so it's the one key where a key-missing scenario has both a legitimate root cause (legacy persisted blob) and a safe fallback (leave the field untouched).Customer mitigation (no patch required)
Customers hitting this on the current BC28 build can self-mitigate by opening the Shopify Bulk Operations page and running Delete Entries Older Than 7 Days. The deletion bypasses the rollback trigger, removing the stale pre-upgrade rows without re-throwing. Bulk operations queued after the upgrade are written in the new layout and are unaffected.
Scope
Main only — rare upgrade-window race condition with a self-service mitigation available. Not backported.
Work Item(s)
Fixes AB#637250