You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In PR #700 (merged on Jun 10), a change was introduced to use explode() for parameter validation in CatalogItemsV20220401Api. While the PR author noted "it would make more sense if these parameters were typed as arrays to begin with, but making a change like that would break existing implementations", the implementation actually caused the breaking change it aimed to avoid.
This change was released in v5.10.3 and is now strictly enforcing string type for its second parameter in explode(). Previously, this parameter accepted both string and array types, which allowed for more flexible usage. This change has caused TypeErrors in our production environment:
TypeError: explode(): Argument #2 ($string) must be of type string, array given
Requiring extensive code revisions in existing implementations
Affecting marketplace ID processing functionality
Breaking existing library integrations that were using array inputs
Expected Behavior
The function should maintain backwards compatibility by accepting both string and array types as it did previously. This flexibility was a key feature that many implementations rely on and was the original intention in PR #700.
Current Behavior
The function now strictly accepts only string type, throwing a TypeError when an array is provided, which contradicts the PR #700's goal of avoiding breaking changes.
Test Coverage: This incident highlights the importance of comprehensive test coverage for parameter types and edge cases
Breaking Changes: Such type restrictions should be introduced with proper deprecation notices
Proposed Solution
Modify the parameter type definition to accept both string and array types, restoring the functionality that PR #700 intended to preserve. This ensures existing implementations continue to work without requiring extensive revisions.
Additional Context
The issue occurs in:
/vendor/jlevers/selling-partner-api/lib/Api/CatalogItemsV20220401Api.php at line 856
The change affects three parameter validations:
marketplace_ids
identifiers
keywords
This issue demonstrates the critical importance of:
Maintaining backwards compatibility as explicitly intended
Comprehensive test coverage
Careful consideration of type restrictions in public APIs
The text was updated successfully, but these errors were encountered:
burakaktna
added a commit
to burakaktna/selling-partner-api
that referenced
this issue
Dec 16, 2024
Issue Description
In PR #700 (merged on Jun 10), a change was introduced to use
explode()
for parameter validation inCatalogItemsV20220401Api
. While the PR author noted "it would make more sense if these parameters were typed as arrays to begin with, but making a change like that would break existing implementations", the implementation actually caused the breaking change it aimed to avoid.This change was released in v5.10.3 and is now strictly enforcing string type for its second parameter in
explode()
. Previously, this parameter accepted both string and array types, which allowed for more flexible usage. This change has caused TypeErrors in our production environment:TypeError: explode(): Argument #2 ($string) must be of type string, array given
Impact
Expected Behavior
The function should maintain backwards compatibility by accepting both string and array types as it did previously. This flexibility was a key feature that many implementations rely on and was the original intention in PR #700.
Current Behavior
The function now strictly accepts only string type, throwing a TypeError when an array is provided, which contradicts the PR #700's goal of avoiding breaking changes.
Why This Matters
Proposed Solution
Modify the parameter type definition to accept both string and array types, restoring the functionality that PR #700 intended to preserve. This ensures existing implementations continue to work without requiring extensive revisions.
Additional Context
The issue occurs in:
/vendor/jlevers/selling-partner-api/lib/Api/CatalogItemsV20220401Api.php at line 856
The change affects three parameter validations:
This issue demonstrates the critical importance of:
The text was updated successfully, but these errors were encountered: