Page MenuHomeSoftware Heritage
Differential D4515

api/serializers: Add Exception type encoder
ClosedPublic
Actions

Authored by anlambert on Nov 18 2020, 6:13 PM.

Details

Reviewers
vlorentz
ardumont
Group Reviewers
Reviewers
Commits
rDCOREf9619fb188c3: api/serializers: Add Exception type encoder and decoder
Summary

I stumbled across that error while working on the Web API POC based on OpenAPI
due to uffizi.internal.softwareheritage.org host currently not available.

INFO:werkzeug:127.0.0.1 - - [18/Nov/2020 17:33:25] "POST /content/data HTTP/1.1" 500 -
Traceback (most recent call last):
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/connection.py", line 160, in _new_conn
    (self._dns_host, self.port), self.timeout, **extra_kw
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/util/connection.py", line 84, in create_connection
    raise err
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/util/connection.py", line 74, in create_connection
    sock.connect(sa)
OSError: [Errno 113] No route to host

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/connectionpool.py", line 677, in urlopen
    chunked=chunked,
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/connectionpool.py", line 392, in _make_request
    conn.request(method, url, **httplib_request_kw)
  File "/usr/lib/python3.7/http/client.py", line 1244, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1290, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1239, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1026, in _send_output
    self.send(msg)
  File "/usr/lib/python3.7/http/client.py", line 966, in send
    self.connect()
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/connection.py", line 187, in connect
    conn = self._new_conn()
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/connection.py", line 172, in _new_conn
    self, "Failed to establish a new connection: %s" % e
urllib3.exceptions.NewConnectionError: <urllib3.connection.HTTPConnection object at 0x7fcf9a993ba8>: Failed to establish a new connection: [Errno 113] No route to host

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/requests/adapters.py", line 449, in send
    timeout=timeout
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/connectionpool.py", line 767, in urlopen
    **response_kw
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/connectionpool.py", line 767, in urlopen
    **response_kw
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/connectionpool.py", line 767, in urlopen
    **response_kw
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/connectionpool.py", line 727, in urlopen
    method, url, error=e, _pool=self, _stacktrace=sys.exc_info()[2]
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/urllib3/util/retry.py", line 439, in increment
    raise MaxRetryError(_pool, url, error or ResponseError(cause))
urllib3.exceptions.MaxRetryError: HTTPConnectionPool(host='uffizi.internal.softwareheritage.org', port=5003): Max retries exceeded with url: /content/get (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7fcf9a993ba8>: Failed to establish a new connection: [Errno 113] No route to host'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/anlambert/swh/swh-environment/swh-core/swh/core/api/__init__.py", line 254, in raw_verb
    return getattr(self.session, verb)(self._url(endpoint), **opts)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/requests/sessions.py", line 578, in post
    return self.request('POST', url, data=data, json=json, **kwargs)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/requests/sessions.py", line 530, in request
    resp = self.send(prep, **send_kwargs)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/requests/sessions.py", line 643, in send
    r = adapter.send(request, **kwargs)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/requests/adapters.py", line 516, in send
    raise ConnectionError(e, request=request)
requests.exceptions.ConnectionError: HTTPConnectionPool(host='uffizi.internal.softwareheritage.org', port=5003): Max retries exceeded with url: /content/get (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7fcf9a993ba8>: Failed to establish a new connection: [Errno 113] No route to host'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/flask/app.py", line 1950, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/flask/app.py", line 1936, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "<decorator-gen-10>", line 2, in content_get_data
    
  File "/home/anlambert/swh/swh-environment/swh-core/swh/core/api/negotiation.py", line 147, in _negotiate
    return f.negotiator(*args, **kwargs)
  File "/home/anlambert/swh/swh-environment/swh-core/swh/core/api/negotiation.py", line 81, in __call__
    result = self.func(*args, **kwargs)
  File "/home/anlambert/swh/swh-environment/swh-core/swh/core/api/__init__.py", line 456, in _f
    return obj_meth(**kw)
  File "/home/anlambert/swh/swh-environment/swh-storage/swh/storage/metrics.py", line 24, in d
    return f(*a, **kw)
  File "/home/anlambert/swh/swh-environment/swh-storage/swh/storage/postgresql/storage.py", line 274, in content_get_data
    return self.objstorage.content_get(content)
  File "/home/anlambert/swh/swh-environment/swh-storage/swh/storage/objstorage.py", line 41, in content_get
    data = self.objstorage.get(obj_id)
  File "/home/anlambert/swh/swh-environment/swh-objstorage/swh/objstorage/multiplexer/multiplexer_objstorage.py", line 278, in get
    return storage.get(obj_id)
  File "/home/anlambert/swh/swh-environment/swh-objstorage/swh/objstorage/multiplexer/multiplexer_objstorage.py", line 105, in call
    return self.get_result_from_mailbox(mailbox)
  File "/home/anlambert/swh/swh-environment/swh-objstorage/swh/objstorage/multiplexer/multiplexer_objstorage.py", line 83, in get_result_from_mailbox
    raise result["result"] from None
  File "/home/anlambert/swh/swh-environment/swh-objstorage/swh/objstorage/multiplexer/multiplexer_objstorage.py", line 28, in run
    ret = getattr(self.storage, command)(*args, **kwargs)
  File "/home/anlambert/swh/swh-environment/swh-objstorage/swh/objstorage/api/client.py", line 54, in get
    return self._proxy.post("content/get", {"obj_id": obj_id})
  File "/home/anlambert/swh/swh-environment/swh-core/swh/core/api/__init__.py", line 272, in post
    **opts,
  File "/home/anlambert/swh/swh-environment/swh-core/swh/core/api/__init__.py", line 256, in raw_verb
    raise self.api_exception(e)
swh.objstorage.exc.ObjStorageAPIError: An unexpected error occurred in the api backend: HTTPConnectionPool(host='uffizi.internal.softwareheritage.org', port=5003): Max retries exceeded with url: /content/get (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7fcf9a993ba8>: Failed to establish a new connection: [Errno 113] No route to host'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/flask/app.py", line 2464, in __call__
    return self.wsgi_app(environ, start_response)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/flask/app.py", line 2450, in wsgi_app
    response = self.handle_exception(e)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/flask/app.py", line 1867, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/flask/_compat.py", line 39, in reraise
    raise value
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/flask/app.py", line 2447, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/flask/app.py", line 1952, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/flask/app.py", line 1822, in handle_user_exception
    return handler(e)
  File "/home/anlambert/swh/swh-environment/swh-storage/swh/storage/api/server.py", line 48, in my_error_handler
    return error_handler(exception, encode_data)
  File "/home/anlambert/swh/swh-environment/swh-core/swh/core/api/__init__.py", line 399, in error_handler
    response = encoder(exception_to_dict(exception))
  File "/home/anlambert/swh/swh-environment/swh-core/swh/core/api/__init__.py", line 375, in encode_data_server
    encoded_data = ENCODERS[content_type](data, extra_encoders=extra_type_encoders)
  File "/home/anlambert/swh/swh-environment/swh-core/swh/core/api/serializers.py", line 228, in msgpack_dumps
    return msgpack.packb(data, use_bin_type=True, default=encode_types)
  File "/home/anlambert/.virtualenvs/swh/lib/python3.7/site-packages/msgpack/__init__.py", line 35, in packb
    return Packer(**kwargs).pack(o)
  File "msgpack/_packer.pyx", line 286, in msgpack._cmsgpack.Packer.pack
    
  File "msgpack/_packer.pyx", line 292, in msgpack._cmsgpack.Packer.pack
    
  File "msgpack/_packer.pyx", line 289, in msgpack._cmsgpack.Packer.pack
    
  File "msgpack/_packer.pyx", line 225, in msgpack._cmsgpack.Packer._pack
    
  File "msgpack/_packer.pyx", line 225, in msgpack._cmsgpack.Packer._pack
    
  File "msgpack/_packer.pyx", line 258, in msgpack._cmsgpack.Packer._pack
    
  File "msgpack/_packer.pyx", line 283, in msgpack._cmsgpack.Packer._pack
    
TypeError: can not serialize 'ConnectionError' object

A RemoteException could not be properly serialized due to the corner case
that the wrapped expection has been constructed with another exception
argument (here ObjStorageAPIError(ConnectionError())).

So handle that special case by adding a dedicated encoder to avoid remote
exception serialization error.

Diff Detail

Repository
rDCORE Foundations and core functionalities
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.Nov 18 2020, 6:13 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptNov 18 2020, 6:13 PM
swh-public-ci added a comment.Nov 18 2020, 6:14 PM

Build is green

Patch application report for D4515 (id=16009)

Rebasing onto 12b0e76def...

Current branch diff-target is up to date.
Changes applied before test
commit 99eef1642dd508de54c5433d83c28b1a9fc6b69f
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Nov 18 18:04:28 2020 +0100

    api/serializers: Add Exception type encoder
    
    An exception can be constructed with another exception argument so handle
    that special case by adding a dedicated encoder to avoid remote exception
    serialization error.

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

Harbormaster completed remote builds in B17216: Diff 16009.Nov 18 2020, 6:14 PM
ardumont added a subscriber: ardumont.Nov 19 2020, 9:37 AM

lgtm

a question inline.

swh/core/api/serializers.py
75

Why not directly:

(Exception, "exception", exception_to_dict)

?

Also, from where does exception-to-dict come from?

ardumont accepted this revision.Nov 19 2020, 9:37 AM
This revision is now accepted and ready to land.Nov 19 2020, 9:37 AM
vlorentz added a subscriber: vlorentz.Nov 19 2020, 9:39 AM

It's missing a decoder; with this diff, exceptions get turned into dicts but not back into exceptions.

And this also means that when you do raise X(Y(z)), X gets serialized differently (by the exception-handling of the RPC) then Y (by the encoder thing), it would be better to use the same mechanism for both. (eg. by using the encoder/decoder in the exception-handling of the RPC)

And naming nitpick: that's not an exception chain; an exception chain would be raise X from Y. That's just an exception object that isn't directly the one being raised.

vlorentz requested changes to this revision.Nov 19 2020, 9:39 AM
This revision now requires changes to proceed.Nov 19 2020, 9:39 AM
ardumont resigned from this revision.Nov 19 2020, 9:40 AM
vlorentz added a comment.Nov 19 2020, 9:40 AM
In D4515#112747, @vlorentz wrote:

And this also means that when you do raise X(Y(z)), X gets serialized differently (by the exception-handling of the RPC) then Y (by the encoder thing), it would be better to use the same mechanism for both. (eg. by using the encoder/decoder in the exception-handling of the RPC)

Forget this part, I didn't notice you reused exception_to_dict

anlambert added a comment.Nov 19 2020, 11:19 AM

It's missing a decoder; with this diff, exceptions get turned into dicts but not back into exceptions.

I wasn't sure if it was needed for that case. I will add the decoder.

And naming nitpick: that's not an exception chain; an exception chain would be raise X from Y. That's just an exception object that isn't directly the one being raised.

I know but could not find a better name that is not too long. I would go for exception_exc_arg then.

swh/core/api/serializers.py
75

oh right, exception_to_dict is defined at the end of the file

anlambert updated this revision to Diff 16050.Nov 19 2020, 3:16 PM

Update: Add Exception decoder, update tests and naming

swh-public-ci added a comment.Nov 19 2020, 3:17 PM

Build is green

Patch application report for D4515 (id=16050)

Rebasing onto 12b0e76def...

Current branch diff-target is up to date.
Changes applied before test
commit 0f827f17a5cdad3c67222866f9333e27caebd1c1
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Nov 18 18:04:28 2020 +0100

    api/serializers: Add Exception type encoder and decoder
    
    An exception can be constructed with another exception argument so handle
    that special case by adding a dedicated encoder and decoder to avoid remote
    exception serialization error.

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

Harbormaster completed remote builds in B17254: Diff 16050.Nov 19 2020, 3:17 PM
vlorentz added inline comments.Nov 19 2020, 3:23 PM
swh/core/api/serializers.py
46

that's redundant now (complete document is something like {"swhtype": "exception", "value": {"exception": { ... } } }). Could you remove the "exception":?

It may need some tweaking in the exception handling

anlambert added inline comments.Nov 19 2020, 3:27 PM
swh/core/api/serializers.py
46

I did that in a first place but was not sure about its impact so I reverted it.

If you say it is not needed, I will update the diff.

anlambert updated this revision to Diff 16063.Nov 19 2020, 3:57 PM

Update: Simplify encoded exception dict

swh-public-ci added a comment.Nov 19 2020, 3:58 PM

Build is green

Patch application report for D4515 (id=16063)

Rebasing onto 12b0e76def...

Current branch diff-target is up to date.
Changes applied before test
commit cbe73e945ae225b36b7bc09e1f953e07e8f22a37
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Nov 18 18:04:28 2020 +0100

    api/serializers: Add Exception type encoder and decoder
    
    An exception can be constructed with another exception argument so handle
    that special case by adding a dedicated encoder and decoder to avoid remote
    exception serialization error.

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

Harbormaster completed remote builds in B17267: Diff 16063.Nov 19 2020, 3:58 PM
vlorentz added a comment.Nov 19 2020, 4:05 PM

The code you changedin swh/core/api/__init__.py isn't covered by tests (that's on me), did you check it still works by running swh-storage tests?

swh/core/api/__init__.py
335–344

you should add elif "exception" in data: exception = RemoteException(payload=data["exception"], response=response), so we can upgrade clients first, then servers, without shutting everything down.

Then we can remove them.

anlambert added a comment.Nov 19 2020, 5:11 PM
In D4515#113121, @vlorentz wrote:

The code you changedin swh/core/api/__init__.py isn't covered by tests (that's on me), did you check it still works by running swh-storage tests?

I just ran storage tests with the current changes and they passed.

swh/core/api/__init__.py
335–344

Ack, will update.

anlambert updated this revision to Diff 16070.Nov 19 2020, 5:12 PM

Update: Address latest @vlorentz comment

swh-public-ci added a comment.Nov 19 2020, 5:14 PM

Build is green

Patch application report for D4515 (id=16070)

Rebasing onto 12b0e76def...

Current branch diff-target is up to date.
Changes applied before test
commit 0361606ac9d5916af2980026e8f9c1a3d5812e62
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Nov 18 18:04:28 2020 +0100

    api/serializers: Add Exception type encoder and decoder
    
    An exception can be constructed with another exception argument so handle
    that special case by adding a dedicated encoder and decoder to avoid remote
    exception serialization error.

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

Harbormaster completed remote builds in B17272: Diff 16070.Nov 19 2020, 5:14 PM
vlorentz accepted this revision.Nov 20 2020, 9:50 AM
This revision is now accepted and ready to land.Nov 20 2020, 9:50 AM
anlambert updated this revision to Diff 16095.Nov 20 2020, 10:53 AM

Rebase

swh-public-ci added a comment.Nov 20 2020, 10:54 AM

Build is green

Patch application report for D4515 (id=16095)

Rebasing onto e2bd9c319b...

Current branch diff-target is up to date.
Changes applied before test
commit f9619fb188c36dd9ce12cfcca493f3268e829172
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Nov 18 18:04:28 2020 +0100

    api/serializers: Add Exception type encoder and decoder
    
    An exception can be constructed with another exception argument so handle
    that special case by adding a dedicated encoder and decoder to avoid remote
    exception serialization error.

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

Harbormaster completed remote builds in B17293: Diff 16095.Nov 20 2020, 10:54 AM
Closed by commit rDCOREf9619fb188c3: api/serializers: Add Exception type encoder and decoder (authored by anlambert). · Explain WhyNov 20 2020, 10:58 AM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDCOREf9619fb188c3: api/serializers: Add Exception type encoder and decoder.
ardumont mentioned this in D4553: vault: test_server: Fix exception structure.Nov 23 2020, 9:55 AM

Revision Contents
Changeset List

PathSize
swh/
core/
api/
16 lines
31 lines
tests/
4 lines
13 lines
21 lines

Diff 16096

View Options

swh/core/api/__init__.py

Loading...
View Options

swh/core/api/serializers.py

Loading...
View Options

swh/core/api/tests/test_async.py

Loading...
View Options

swh/core/api/tests/test_rpc_client_server.py

Loading...
View Options

swh/core/api/tests/test_serializers.py

Loading...
About · Developer information · Legal