Page MenuHomeSoftware Heritage
Differential D4466

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

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

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T2768: unbreak swh-graph CI
Commits
rDGRPH262432b1295d: server/app: Fix aiohttp >= 3.7 exception related errors
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 Compressed graph representation
Branch
fix-aiohttp-exception-construct
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17058
Build 26329: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 26328: arc lint + arc unit

Event Timeline

anlambert created this revision.Nov 12 2020, 3:18 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptNov 12 2020, 3:18 PM
anlambert updated this revision to Diff 15848.Nov 12 2020, 3:19 PM

Add task reference in commit message.

ardumont added a subscriber: ardumont.Nov 12 2020, 3:19 PM
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.Nov 12 2020, 3:19 PM

one question inline

otherwise, lgtm

and thanks for fixing it.

This revision is now accepted and ready to land.Nov 12 2020, 3:19 PM
swh-public-ci added a comment.Nov 12 2020, 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.

Harbormaster completed remote builds in B17058: Diff 15847.Nov 12 2020, 3:19 PM
swh-public-ci added a comment.Nov 12 2020, 3:21 PM

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.

Harbormaster completed remote builds in B17059: Diff 15848.Nov 12 2020, 3:21 PM
anlambert added inline comments.Nov 12 2020, 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.

Closed by commit rDGRPH262432b1295d: server/app: Fix aiohttp >= 3.7 exception related errors (authored by anlambert). · Explain WhyNov 12 2020, 3:29 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDGRPH262432b1295d: server/app: Fix aiohttp >= 3.7 exception related errors.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
graph/
server/
14 lines37%
Loading...

Diff 15847

View Options

swh/graph/server/app.py

Loading...
About · Developer information · Legal