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

Error Handling for common conditions #37

Open
nirmay opened this issue May 17, 2022 · 13 comments
Open

Error Handling for common conditions #37

nirmay opened this issue May 17, 2022 · 13 comments

Comments

@nirmay
Copy link

nirmay commented May 17, 2022

Just returning the error with message "Item quantity unavailable" is not sufficient for BAP.
In case, there are many items added to the cart, BAP will need to present to the user, which item(s) are low on quantity.

I would like to propose, along with error, BPP will return following in the on_init payload, along with "error" object,

message.order.items (which is an array) and BPP sends back current quantity as per below,

{ "id": "73661421", "quantity": { "count": 0 } },

@BLR-0118 pls review,

@EkanshKr
Copy link

This should work.
We should keep the quote.breakup too in the message.order. For example - There are 4 products in the /init call and for 1 product we may get quantity mismatch and for another one we may get price update error. It's better to cover these two scenarios in one /on_init request (quantity unavailable and price update can happen for the same product too).
As we have “@ondc/org/item-qty” in the quote.breakup object now, so BPPs can send the price according to available quantity and can mention the qty in this field too.
@BLR-0118 @nirmay any challenges if we add quote in this?

@mahoriR
Copy link

mahoriR commented May 19, 2022

@nirmay This is good.

So if i have to generalize this -

In case of this error - on_init the response will have the actual available quantities (less then or equal to what is asked In init).
So if requestor asks for 2 items , 3 quantity each , we could respond with -
[{ "id": "73661421", "quantity": { "count": 1 } },{ "id": "73661421", "quantity": { "count": 3 } },]
and an error for insufficient quantity.

This is right ?

@mahoriR
Copy link

mahoriR commented May 19, 2022

@EkanshKr While the order object can have multiple changes (prices, quantities) but error object today doesn't have a way to convey multiple errors. That would be a problem right.
@BLR-0118

@nirmay
Copy link
Author

nirmay commented May 19, 2022

@nirmay This is good.

So if i have to generalize this -

In case of this error - on_init the response will have the actual available quantities (less then or equal to what is asked In init). So if requestor asks for 2 items , 3 quantity each , we could respond with - [{ "id": "73661421", "quantity": { "count": 1 } },{ "id": "73661421", "quantity": { "count": 3 } },] and an error for insufficient quantity.

This is right ?

In my proposal, the items array should include only those items that fail the inventory quantity check.
if buyer orders 10 items and 2 of them fail the check then only those are returned in the on_init response inside the items array.

@VidyaGF
Copy link

VidyaGF commented May 19, 2022

Does this cover if we have two error IDs , quantity error and price error , we will need to display multiple error messages for 2 different items in the same checkout page.

@nirmay
Copy link
Author

nirmay commented May 20, 2022

Does this cover if we have two error IDs , quantity error and price error , we will need to display multiple error messages for 2 different items in the same checkout page.

good point, suggest you file a separate bug to address that.

@EkanshKr
Copy link

I am not sure but currently i don't think that we get any price update error object. Buyer app checks the price at their end and the price what we received in breakup object of /on_init response. do we need different error code for price update?
By Adding breakup object also in the case of quantity error , buyer app can check the price update and quantity in one /on_init response.
@nirmay @VidyaGF @mahoriR

@VidyaGF
Copy link

VidyaGF commented May 20, 2022

I was talking more of making the error object generic so that we can handle multiple errors and item-wise , so that anytime in future, different types of errors can be reported. Buyer app can read list of errors and make provision to display it appropriately to the user.

@BLR-0118
Copy link
Member

@nirmay @mahoriR - agree that we can use this approach only to indicate inventory quantity check failing;
@EkanshKr - price update by seller - is that an error?
Suggest we go ahead with the original reason for which this issue was filed i.e. inventory quantity check failing. Not sure how we can handle multiple errors in the same error object?

@EkanshKr
Copy link

EkanshKr commented Jul 4, 2022

@nirmay @BLR-0118 @VidyaGF @mahoriR
sharing the on_init response for this issue as discussed with Supriyo and Mayuresh -

https://docs.google.com/document/d/19dHfFegVwWIYdnVvE8uUXPZsTl9MjOLjsUv8TAXU5Yw/edit?usp=sharing

@BLR-0118
Copy link
Member

BLR-0118 commented Jul 5, 2022

this should work. Can everyone enable this @nirmay @mahoriR @VidyaGF

Also, @EkanshKr - Paytm will have a generic error handler to show these types of messages?

@EkanshKr
Copy link

EkanshKr commented Jul 6, 2022

@BLR-0118 Yes Supriyo , we have generic error handler to show these messages and we are showing the other error messages like "delivery agent not available" , "price has been updated" through that handler only.

@BLR-0118 BLR-0118 changed the title Need item details for error "40002 - Item quantity unavailable" Error Handling for common conditions Aug 2, 2022
@BLR-0118
Copy link
Member

If > 1 items in error, send an error code such as "mismatch between what is selected and what is available" and for Paytm / other buyer apps to highlight items where there is an issue. If this works, I can define the error code.
@EkanshKr @mahoriR @nirmay @VidyaGF @akil4n

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

No branches or pull requests

5 participants