Page MenuHomeSoftware Heritage

server/app: Fix aiohttp >= 3.7 exception related errors
ClosedPublic

Authored by anlambert on Thu, Nov 12, 3:18 PM.

Details

Summary

Finally found the bug in swh-graph since the bump to aiohttp 3.7.

Using pytest -sv --log-cli-level DEBUG --swh/graph/tests/test_api_client.py::test_param_validation,
the following stack trace is displayed:

ERROR    aiohttp.server:web_protocol.py:393 Unhandled exception
Traceback (most recent call last):
  File "/home/anlambert/swh/swh-environment/swh-graph/swh/graph/server/app.py", line 61, in node_of_swhid
    return self.backend.swhid2node[swhid]
  File "/home/anlambert/swh/swh-environment/swh-graph/swh/graph/swhid.py", line 297, in __getitem__
    return self._find(swhid_str)[0]  # return element, ignore position
  File "/home/anlambert/swh/swh-environment/swh-graph/swh/graph/swhid.py", line 285, in _find
    raise KeyError(swhid_str)
KeyError: 'swh:1:ori:fff0000000000000000000000000000000000021'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/aiohttp/web_protocol.py", line 422, in _handle_request
    resp = await self._request_handler(request)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/aiohttp/web_app.py", line 499, in _handle
    resp = await handler(request)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/aiohttp/web_middlewares.py", line 118, in impl
    return await handler(request)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/aiohttp_utils/negotiation.py", line 216, in middleware
    response = yield from handler(request)
  File "/home/anlambert/swh/swh-environment/swh-core/swh/core/api/asynchronous.py", line 71, in middleware_handler
    return await handler(request)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/aiohttp/web_urldispatcher.py", line 948, in _iter
    resp = await method()
  File "/home/anlambert/swh/swh-environment/swh-graph/swh/graph/server/app.py", line 128, in get
    await self.prepare_response()
  File "/home/anlambert/swh/swh-environment/swh-graph/swh/graph/server/app.py", line 165, in prepare_response
    self.src_node = self.node_of_swhid(src)
  File "/home/anlambert/swh/swh-environment/swh-graph/swh/graph/server/app.py", line 63, in node_of_swhid
    raise aiohttp.web.HTTPNotFound(body=f"SWHID not found: {swhid}")
aiohttp.web_exceptions.HTTPNotFound: Not Found

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/aiohttp/web_protocol.py", line 487, in start
    resp, reset = await task
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/aiohttp/web_protocol.py", line 428, in _handle_request
    status=exc.status, reason=exc.reason, text=exc.text, headers=exc.headers
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/aiohttp/web_response.py", line 650, in text
    return self._body.decode(self.charset or "utf-8")
AttributeError: 'StringPayload' object has no attribute 'decode'

Since aiohttp 3.7, the internal handling of aiohttp.web.HTTPException has changed
and a aiohttp.web_response.Response is now constructed from the exception text
then returned (see reference code).

aiohttp exception constructions in swh-graph were passing error message through
the body keyword parameter instead of the text one, leading to an error related
to unicode decoding as body is expected to be bytes.

Using the text keyword parameter when constructing aiohttp exceptions now ensures
that response body will be properly encoded and thus avoid the decoding errors.

Closes T2768

Diff Detail

Repository
rDGRPH Graph service
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

anlambert created this revision.Thu, Nov 12, 3:18 PM
anlambert updated this revision to Diff 15848.Thu, Nov 12, 3:19 PM

Add task reference in commit message.

ardumont added inline comments.
swh/graph/server/app.py
63

do we need to enforce the variable's name?

(smells like it will get back biting us again next time it got changed again)

ardumont accepted this revision.Thu, Nov 12, 3:19 PM

one question inline

otherwise, lgtm

and thanks for fixing it.

This revision is now accepted and ready to land.Thu, Nov 12, 3:19 PM

Build is green

Patch application report for D4466 (id=15847)

Rebasing onto 8b7c01d861...

Current branch diff-target is up to date.
Changes applied before test
commit 2a4740fc5c0ed70e02c745fafd86c8ac7f9f5099
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Nov 12 15:09:22 2020 +0100

    server/app: Fix aiohttp >= 3.7 exception related errors
    
    Since aiohttp 3.7, the internal handling of aiohttp.web.HTTPException has changed
    and a aiohttp.web_response.Response is now constructed from the exception text
    then returned.
    
    aiohttp exception constructions in swh-graph were passing error message through
    the body keyword parameter instead of the text one, leading to an error related
    to unicode decoding as body is expected to be bytes.
    
    Using the text keyword parameter when constructing aiohttp exceptions now ensures
    that response body will be properly encoded.

See https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/59/ for more details.

Build is green

Patch application report for D4466 (id=15848)

Rebasing onto 8b7c01d861...

Current branch diff-target is up to date.
Changes applied before test
commit 262432b1295d225075c99f58a96ad0cb6248e72a
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Nov 12 15:09:22 2020 +0100

    server/app: Fix aiohttp >= 3.7 exception related errors
    
    Since aiohttp 3.7, the internal handling of aiohttp.web.HTTPException has changed
    and a aiohttp.web_response.Response is now constructed from the exception text
    then returned.
    
    aiohttp exception constructions in swh-graph were passing error message through
    the body keyword parameter instead of the text one, leading to an error related
    to unicode decoding as body is expected to be bytes.
    
    Using the text keyword parameter when constructing aiohttp exceptions now ensures
    that response body will be properly encoded.
    
    Closes T2768

See https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/60/ for more details.

anlambert added inline comments.Thu, Nov 12, 3:26 PM
swh/graph/server/app.py
63

do we need to enforce the variable's name?

Not sure how we could do that apart wrapping the construction of the aiohttp exceptions in a function
but I think using the text parameter seems the most obvious here.