Fix error decoding of string body #31265

kldtz · 2023-07-24T12:34:23Z

If error_body is a string (which may happen), calling iter() on it will result in: AttributeError: 'str' object has no attribute 'iter'. Use the body as message instead.

This error appeared when I configured a wrong endpoint that returned an HTML error response. The AttributeError was hiding the actual error, which made debugging the problem harder.

All SDK Contribution checklist:

The pull request does not introduce [breaking changes]
CHANGELOG is updated for new features, bug fixes or other significant changes.
I have read the contribution guidelines.

General Guidelines and Best Practices

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

If error_body is a string, calling iter() on it will result in: AttributeError: 'str' object has no attribute 'iter'. Use the body as message instead.

github-actions · 2023-07-24T12:34:57Z

Thank you for your contribution @kldtz! We will review the pull request and get back to you soon.

annatisch · 2023-07-24T22:44:20Z

Thanks so much @kldtz!
Could you please provide more details (e.g. a code snippet) on the scenario where you encountered this error?
Then we can add a test to repro this so that the new behaviour can be properly accounted for to prevent any future regressions :)

kldtz · 2023-07-25T06:23:03Z

Hi @annatisch, sure:

client = TableClient(
    # wrong endpoint that responds with HTML
    endpoint="https://<storage>.z6.web.core.windows.net/",
    credential=DefaultAzureCredential(),
    table_name="table-name"
)
client.submit_transaction([
    (
        "upsert",
        {
            "PartitionKey": "test-partition",
            "RowKey": "test-key",
            "name": "test-name",
        },
    )
])

I just saw that in other places _process_table_error is called, which deals with the AttributeError, but _batch_send calls _decode_error directly. Catching the AttributeError and raising the original error would be an alternative fix.

YalinLi0312 · 2023-07-25T19:54:19Z

Hi @kldtz , I got ServiceRequestError instead. Here's the code snippet I tried:

client = TableClient(
            # wrong endpoint that responds with HTML
            endpoint="https://<tables_storage_account_name>.z6.web.core.windows.net/",
            credential=DefaultAzureCredential(),
            table_name="table-name"
        )
        with pytest.raises(ServiceRequestError) as ex:
            client.submit_transaction([
                (
                    "upsert",
                    {
                        "PartitionKey": "test-partition",
                        "RowKey": "test-key",
                        "name": "test-name",
                    },
                )
            ])
        assert "Failed to establish a new connection" in str(ex.value)

Do you know if I missed anything?

kldtz · 2023-07-26T06:01:02Z

Hi @YalinLi0312 ,

you can try this snippet, where I monkey-patched the run method:

from functools import partial
from typing import Any

from azure.core.pipeline import HTTPRequestType, PipelineResponse, HTTPResponseType
from azure.core.pipeline.transport._requests_basic import RequestsTransportResponse
from azure.data.tables import TableClient
from azure.identity import DefaultAzureCredential
from requests import Response


def patch_run(self, request: HTTPRequestType, **kwargs: Any) -> PipelineResponse[HTTPRequestType, HTTPResponseType]:
    response = Response()
    response.status_code = 405
    response._content = b"<!DOCTYPE html><html><head><title>UnsupportedHttpVerb</title></head><body><h1>The resource doesn't support specified Http Verb.</h1><p><ul><li>HttpStatusCode: 405</li><li>ErrorCode: UnsupportedHttpVerb</li><li>RequestId : 98adf858-a01e-0071-2580-bfe811000000</li><li>TimeStamp : 2023-07-26T05:19:26.9825582Z</li></ul></p></body></html>"
    response.url = 'https://<storage>.z6.web.core.windows.net/$batch'
    response.headers = {
        "x-ms-error-code": "UnsupportedHttpVerb",
        "content-type": "text/html"
    }
    return PipelineResponse(
        http_request=None,
        http_response=RequestsTransportResponse(
            requests_response=response,
            request=None,
        ),
        context=None,
    )


if __name__ == '__main__':
    client = TableClient(
        endpoint="https://<storage>.z6.web.core.windows.net/",
        credential=DefaultAzureCredential(),
        table_name="syncenabled"
    )
    client._client._client._pipeline.run = partial(patch_run, client)
    client.submit_transaction([
        (
            "upsert",
            {
                "PartitionKey": "test-partition",
                "RowKey": "test-key",
                "name": "test-name",
            },
        )
    ])

If the content type is text/html the method deserialize_from_http_generics returns a string, which results in the AttributeError. If you're not able to reproduce it, feel free to close the PR.

YalinLi0312 · 2023-07-27T18:17:52Z

Hi @YalinLi0312 ,

you can try this snippet, where I monkey-patched the run method:

from functools import partial
from typing import Any

from azure.core.pipeline import HTTPRequestType, PipelineResponse, HTTPResponseType
from azure.core.pipeline.transport._requests_basic import RequestsTransportResponse
from azure.data.tables import TableClient
from azure.identity import DefaultAzureCredential
from requests import Response


def patch_run(self, request: HTTPRequestType, **kwargs: Any) -> PipelineResponse[HTTPRequestType, HTTPResponseType]:
    response = Response()
    response.status_code = 405
    response._content = b"<!DOCTYPE html><html><head><title>UnsupportedHttpVerb</title></head><body><h1>The resource doesn't support specified Http Verb.</h1><p><ul><li>HttpStatusCode: 405</li><li>ErrorCode: UnsupportedHttpVerb</li><li>RequestId : 98adf858-a01e-0071-2580-bfe811000000</li><li>TimeStamp : 2023-07-26T05:19:26.9825582Z</li></ul></p></body></html>"
    response.url = 'https://<storage>.z6.web.core.windows.net/$batch'
    response.headers = {
        "x-ms-error-code": "UnsupportedHttpVerb",
        "content-type": "text/html"
    }
    return PipelineResponse(
        http_request=None,
        http_response=RequestsTransportResponse(
            requests_response=response,
            request=None,
        ),
        context=None,
    )


if __name__ == '__main__':
    client = TableClient(
        endpoint="https://<storage>.z6.web.core.windows.net/",
        credential=DefaultAzureCredential(),
        table_name="syncenabled"
    )
    client._client._client._pipeline.run = partial(patch_run, client)
    client.submit_transaction([
        (
            "upsert",
            {
                "PartitionKey": "test-partition",
                "RowKey": "test-key",
                "name": "test-name",
            },
        )
    ])

If the content type is text/html the method deserialize_from_http_generics returns a string, which results in the AttributeError. If you're not able to reproduce it, feel free to close the PR.

Thank you, I'll try on it!

YalinLi0312 · 2023-08-04T23:56:38Z

Hi @kldtz , I did reproduce the error with the sample you provided. Can you assign me the permission to edit on your branch? I want to add some tests.

kldtz · 2023-08-05T07:26:19Z

@YalinLi0312 As far as I can see, you already have edit rights.

YalinLi0312 · 2023-08-07T20:32:37Z

@YalinLi0312 As far as I can see, you already have edit rights.

It failed due to lacking permission when I tried. Can you assign me the permission? Or do you mind me fixing it in another PR?

kldtz · 2023-08-08T17:20:44Z

"Allow edits by maintainers" is checked. Not sure what else I can do. You can fix it in another PR.

kldtz · 2023-08-20T20:11:27Z

@microsoft-github-policy-service agree

sdk/tables/azure-data-tables/tests/test_table_batch.py

sdk/tables/azure-data-tables/azure/data/tables/_error.py

Co-authored-by: Anna Tisch <[email protected]>

…into kldtz/main

annatisch

Thanks @kldtz, @YalinLi0312!

sdk/tables/azure-data-tables/azure/data/tables/_error.py

Co-authored-by: Anna Tisch <[email protected]>

YalinLi0312 · 2023-09-01T02:28:11Z

Hi @kldtz - we'll prepare a release with this fix on 9/12/2023, please give it a try and welcome to any feedback!

Fix error decoding

d610c16

If error_body is a string, calling iter() on it will result in: AttributeError: 'str' object has no attribute 'iter'. Use the body as message instead.

kldtz requested review from annatisch and YalinLi0312 as code owners July 24, 2023 12:34

github-actions bot added Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. Tables labels Jul 24, 2023

YalinLi0312 added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Jul 24, 2023

YalinLi0312 removed the needs-author-feedback Workflow: More information is needed from author to address the issue. label Aug 1, 2023

YalinLi0312 added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Aug 4, 2023

YalinLi0312 added 2 commits August 17, 2023 16:24

Merge branch 'main' into kldtz/main

f7d95c6

Add sync test

4018fe6

annatisch reviewed Aug 21, 2023

View reviewed changes

sdk/tables/azure-data-tables/tests/test_table_batch.py Show resolved Hide resolved

YalinLi0312 added 4 commits August 31, 2023 13:55

Update fix

2f57d16

Add async test

fb9eab8

Merge branch 'main' into kldtz/main

0caf172

Update CHANGELOG.md

df535d8

annatisch reviewed Aug 31, 2023

View reviewed changes

sdk/tables/azure-data-tables/azure/data/tables/_error.py Outdated Show resolved Hide resolved

Yalin Li and others added 2 commits August 31, 2023 14:14

Update sdk/tables/azure-data-tables/azure/data/tables/_error.py

8a42886

Co-authored-by: Anna Tisch <[email protected]>

Run black

6c891e3

YalinLi0312 added 5 commits August 31, 2023 14:49

Merge branch 'main' of https://github.com/Azure/azure-sdk-for-python …

1256152

…into kldtz/main

Merge branch 'main' of https://github.com/kldtz/azure-sdk-for-python …

c0c7b7b

…into kldtz/main

Fix pylint errors

53f44ed

Fix cspell

b190a3d

Update CHANGELOG.md

da9f831

annatisch approved these changes Sep 1, 2023

View reviewed changes

sdk/tables/azure-data-tables/azure/data/tables/_error.py Outdated Show resolved Hide resolved

Update sdk/tables/azure-data-tables/azure/data/tables/_error.py

174f54e

Co-authored-by: Anna Tisch <[email protected]>

YalinLi0312 enabled auto-merge (squash) September 1, 2023 02:06

YalinLi0312 merged commit 87c886d into Azure:main Sep 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix error decoding of string body #31265

Fix error decoding of string body #31265

kldtz commented Jul 24, 2023 •

edited

Loading

github-actions bot commented Jul 24, 2023

annatisch commented Jul 24, 2023

kldtz commented Jul 25, 2023 •

edited

Loading

YalinLi0312 commented Jul 25, 2023

kldtz commented Jul 26, 2023 •

edited

Loading

YalinLi0312 commented Jul 27, 2023

YalinLi0312 commented Aug 4, 2023

kldtz commented Aug 5, 2023

YalinLi0312 commented Aug 7, 2023

kldtz commented Aug 8, 2023

kldtz commented Aug 20, 2023

annatisch left a comment

YalinLi0312 commented Sep 1, 2023

Fix error decoding of string body #31265

Fix error decoding of string body #31265

Conversation

kldtz commented Jul 24, 2023 • edited Loading

All SDK Contribution checklist:

General Guidelines and Best Practices

Testing Guidelines

github-actions bot commented Jul 24, 2023

annatisch commented Jul 24, 2023

kldtz commented Jul 25, 2023 • edited Loading

YalinLi0312 commented Jul 25, 2023

kldtz commented Jul 26, 2023 • edited Loading

YalinLi0312 commented Jul 27, 2023

YalinLi0312 commented Aug 4, 2023

kldtz commented Aug 5, 2023

YalinLi0312 commented Aug 7, 2023

kldtz commented Aug 8, 2023

kldtz commented Aug 20, 2023

annatisch left a comment

Choose a reason for hiding this comment

YalinLi0312 commented Sep 1, 2023

kldtz commented Jul 24, 2023 •

edited

Loading

kldtz commented Jul 25, 2023 •

edited

Loading

kldtz commented Jul 26, 2023 •

edited

Loading