Error Model aligment with Commonalities Meta-Release Fall24

camaraproject / CarrierBillingCheckOut

Repository to describe, develop, document and test the Carrier Billing Check Out API family

Apache License 2.0

9 stars 9 forks source link

Error Model aligment with Commonalities Meta-Release Fall24 #154

Open PedroDiez opened 1 month ago

PedroDiez commented 1 month ago

Problem description In the Context of Commonalities/Issue 180 and Commonalities/Issue 127. An error model alingment is required.

@bigludo7 also commented in 17th April meeting that could make sense to have some errors being addressed with 422 exception instead of 403 one.

Based on that aligment, it would be required to make proper adaptations in both carrier billing payment and carrier billing refund functionality.

Possible evolution Pending to be proposed

Alternative solution N/A

Additional context N/A

PedroDiez commented 3 weeks ago

Please find attached error model aligment porposal to discuss today (based on Commonalities Aligment PR#213)

CB_Error_Model_Alignment_Proposal.xlsx

cc @bigludo7, @rartych, @eragaji

PedroDiez commented 2 weeks ago

Hi,

@bigludo7, @rartych please can you provide feedback? Based on that we can try to reach a conclusion about updates in error excepctions

bigludo7 commented 2 weeks ago

Hello @PedroDiez Sorry to be late on this. Took a look.

Only one comment : I'm bit skeptical of the propose use of 429 for CARRIER_BILLING.USER_AMOUNT_THRESHOLD_OVERPASSED. For me

429 is used if we got too many 'technical' request from a consumer. I'm not thinking of a business error.
Here we have a business error (for my understanding) - as a carrier billing operator we do not allow consumer to spend more than x euros per month on Carrier billing payment. In this case I will probably more expect a 422 (and the code CARRIER_BILLING.USER_AMOUNT_THRESHOLD_OVERPASSED is perfect).

PedroDiez commented 1 week ago

Hello @PedroDiez Sorry to be late on this. Took a look.

Only one comment : I'm bit skeptical of the propose use of 429 for CARRIER_BILLING.USER_AMOUNT_THRESHOLD_OVERPASSED. For me

429 is used if we got too many 'technical' request from a consumer. I'm not thinking of a business error.

Here we have a business error (for my understanding) - as a carrier billing operator we do not allow consumer to spend more than x euros per month on Carrier billing payment. In this case I will probably more expect a 422 (and the code CARRIER_BILLING.USER_AMOUNT_THRESHOLD_OVERPASSED is perfect).

Understood @bigludo7, makes sense to use 422, also being compliant with the guidelines. For the rest of the proposal, I guess you seems right, doesn't?

bigludo7 commented 1 week ago

@PedroDiez

For the rest of the proposal, I guess you seems right, doesn't?

Yes the rest of the proposal is fine for me :)

PedroDiez commented 1 week ago

Will wait EOD this Thursday to check other comments and moving forward with the updates

PedroDiez commented 1 week ago

Already covered in #152