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

[FIX] payment_mercado_pago: handle mercado pago no body responses #152102

Conversation

OmarAbosamaha
Copy link
Contributor

[FIX] payment_mercado_pago: handle mercado pago no body responses
Issue:
When using a mercado pago invalid access token with extra tabs, we get 403 response from mercado pago without a body which raises and exception while handling this exception we fail to parse the response as it has no body.

line causing the issue:

response_content = response.json()

Steps to reproduce:
1- Enable mercado pago as a payment provider
2- Set a valid access token for mercado pago with extra tabs 3- Go to website
4- Fill the cart
5- Checkout with the cart using mercado pago
6- You see error message of unhandled json parsing error

Solution:
We should wrap parsing the response in a try statement to handle the responses without body

opw-3654133

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@OmarAbosamaha OmarAbosamaha self-assigned this Jan 31, 2024
@robodoo
Copy link
Contributor

robodoo commented Jan 31, 2024

Pull request status dashboard.

@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Jan 31, 2024
@OmarAbosamaha OmarAbosamaha force-pushed the 16.0-opw-3654133-handle-mercado-pago-empty-response-abom branch from f5b85e6 to f9f289c Compare February 1, 2024 10:24
Copy link
Contributor

@nle-odoo nle-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks ok if we don't get a json response and don't have any more info

I can't reproduce with the steps (I get on the https://sandbox.mercadopago.com.mx/ website and I don't get any error). I don't know what extra tabs meant in "access token for mercado pago with extra tabs" so maybe that's why.

If we add information about why the call fails, it would be useful to give it in the error_message.

@OmarAbosamaha
Copy link
Contributor Author

looks ok if we don't get a json response and don't have any more info

I can't reproduce with the steps (I get on the https://sandbox.mercadopago.com.mx/ website and I don't get any error). I don't know what extra tabs meant in "access token for mercado pago with extra tabs" so maybe that's why.

If we add information about why the call fails, it would be useful to give it in the error_message.

Unfortunately we don't get any more information from mercado pago, so that's why.
Also what is meant in "access token for mercado pago with extra tabs" is just add extra spaces at the end of the key

@OmarAbosamaha OmarAbosamaha marked this pull request as ready for review February 1, 2024 13:51
@C3POdoo C3POdoo requested a review from a team February 1, 2024 14:09
Copy link
Contributor

@AntoineVDV AntoineVDV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, when trying to reproduce the issue by padding the access token with extra spaces, I got the following error message:

image

I think it's clear enough, and we should not discard it.

@OmarAbosamaha
Copy link
Contributor Author

Hi, when trying to reproduce the issue by padding the access token with extra spaces, I got the following error message:

image

I think it's clear enough, and we should not discard it.

Yes we're not discarding it. This will stay as is. In order to reproduce this specific bug you need to pad the access token with tab spaces. In that case mercado pago will not respond with that known error. Instead, it will respond with status code 403 only and now we're handling that 403 response.

@AntoineVDV
Copy link
Contributor

Could you share a runbot that has the problem? It works just fine (clear error) for me.

@OmarAbosamaha
Copy link
Contributor Author

Could you share a runbot that has the problem? It works just fine (clear error) for me.

Can you try here: https://57568658-master-all.runbot185.odoo.com/shop/payment

Copy link
Contributor

@AntoineVDV AntoineVDV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok this time I could reproduce. Maybe I didn't pad my token enough 🤷

Comment on lines 75 to 76
error_code = response.status_code
error_message = ''
Copy link
Contributor

@AntoineVDV AntoineVDV Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not try to force dummy error codes and messages into the existing ValidationError text; "information (code 403)" is useless to users.

Instead, we should raise a different ValidationError ("The communication with the API failed. The response is empty. Please verify your access token.") when a ValueError exception is raised. Also, there should be a comment "The API response is sometimes empty when the access token is wrong.".

@OmarAbosamaha OmarAbosamaha force-pushed the 16.0-opw-3654133-handle-mercado-pago-empty-response-abom branch from f9f289c to 0864c79 Compare February 5, 2024 11:02
Issue:
When using a mercado pago invalid access token with extra tabs, we get 403 response from mercado pago without a body
which raises and exception while handling this exception we fail to parse the response as it has no body.

line causing the issue: https://github.com/odoo/odoo/blob/9764e6f7fe39a10f3b04e1764110d8c274d0431a/addons/payment_mercado_pago/models/payment_provider.py#L70

Steps to reproduce:
1- Enable mercado pago as a payment provider
2- Set a valid access token for mercado pago with extra tabs
3- Go to website
4- Fill the cart
5- Checkout with the cart using mercado pago
6- You see error message of unhandled json parsing error

Solution:
We should wrap parsing the response in a try statement to handle the responses without body

opw-3654133
@AntoineVDV AntoineVDV force-pushed the 16.0-opw-3654133-handle-mercado-pago-empty-response-abom branch from 0864c79 to 64acf96 Compare February 5, 2024 12:12
Copy link
Contributor

@AntoineVDV AntoineVDV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robodoo delegate+

@OmarAbosamaha
Copy link
Contributor Author

@robodoo r+

robodoo pushed a commit that referenced this pull request Feb 5, 2024
Issue:
When using a mercado pago invalid access token with extra tabs, we get 403 response from mercado pago without a body
which raises and exception while handling this exception we fail to parse the response as it has no body.

line causing the issue: https://github.com/odoo/odoo/blob/9764e6f7fe39a10f3b04e1764110d8c274d0431a/addons/payment_mercado_pago/models/payment_provider.py#L70

Steps to reproduce:
1- Enable mercado pago as a payment provider
2- Set a valid access token for mercado pago with extra tabs
3- Go to website
4- Fill the cart
5- Checkout with the cart using mercado pago
6- You see error message of unhandled json parsing error

Solution:
We should wrap parsing the response in a try statement to handle the responses without body

opw-3654133

closes #152102

Signed-off-by: Omar Abosamaha (abom) <[email protected]>
@robodoo robodoo closed this Feb 5, 2024
luanjubica pushed a commit to luanjubica/odoo-code that referenced this pull request Feb 14, 2024
Issue:
When using a mercado pago invalid access token with extra tabs, we get 403 response from mercado pago without a body
which raises and exception while handling this exception we fail to parse the response as it has no body.

line causing the issue: https://github.com/odoo/odoo/blob/9764e6f7fe39a10f3b04e1764110d8c274d0431a/addons/payment_mercado_pago/models/payment_provider.py#L70

Steps to reproduce:
1- Enable mercado pago as a payment provider
2- Set a valid access token for mercado pago with extra tabs
3- Go to website
4- Fill the cart
5- Checkout with the cart using mercado pago
6- You see error message of unhandled json parsing error

Solution:
We should wrap parsing the response in a try statement to handle the responses without body

opw-3654133

closes odoo#152102

Signed-off-by: Omar Abosamaha (abom) <[email protected]>
gamarino pushed a commit to numaes/numa-public-odoo that referenced this pull request Feb 18, 2024
Issue:
When using a mercado pago invalid access token with extra tabs, we get 403 response from mercado pago without a body
which raises and exception while handling this exception we fail to parse the response as it has no body.

line causing the issue: https://github.com/odoo/odoo/blob/9764e6f7fe39a10f3b04e1764110d8c274d0431a/addons/payment_mercado_pago/models/payment_provider.py#L70

Steps to reproduce:
1- Enable mercado pago as a payment provider
2- Set a valid access token for mercado pago with extra tabs
3- Go to website
4- Fill the cart
5- Checkout with the cart using mercado pago
6- You see error message of unhandled json parsing error

Solution:
We should wrap parsing the response in a try statement to handle the responses without body

opw-3654133

closes odoo/odoo#152102

Signed-off-by: Omar Abosamaha (abom) <[email protected]>
@fw-bot fw-bot deleted the 16.0-opw-3654133-handle-mercado-pago-empty-response-abom branch February 19, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants