Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed InventoryLevel.Available from int to long #517

Merged
merged 4 commits into from
May 30, 2020

Conversation

clement911
Copy link
Collaborator

I came across in production an inventory_level with a available quantity lower than int.MinValue

image

@clement911 clement911 changed the title Changed InventoryLevel.Available from int to long Changed InventoryLevel.Available from int to long + RequestId in ShopifyException message May 27, 2020
@nozzlegear
Copy link
Owner

Thanks for the PR! I'm not sure I'm comfortable with altering the exception message right now. It's probably a bad practice, but it's likely that some people are checking the message value when catching exceptions, as I used to recommend they do that for rate exceptions. I think it should still be possible to access that request ID if you try to cast a generic exception to a ShopifyException?

… are logged, it is very useful to be able to see the Request-Id for Shopify support to investigate."

This reverts commit aec2d2a.
@clement911 clement911 changed the title Changed InventoryLevel.Available from int to long + RequestId in ShopifyException message Changed InventoryLevel.Available from int to long May 30, 2020
@clement911
Copy link
Collaborator Author

Fair enough.
I've reverted that part.

@nozzlegear nozzlegear merged commit a6c90f9 into nozzlegear:master May 30, 2020
@nozzlegear
Copy link
Owner

Thanks! Published in 5.2.0

@clement911
Copy link
Collaborator Author

Thank you

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.

2 participants