Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Ensure the msg property of HttpResponseException is a string. #7979

Merged
merged 1 commit into from
Jul 29, 2020

Conversation

clokep
Copy link
Member

@clokep clokep commented Jul 29, 2020

While working on #7674 I had missed that the phrase property of a treq responses is bytes, not a string. This gets used to create HttpResponseException, which doesn't match what is expected there.

The phrase gets set as HttpResponseException.msg, which can be JSON serialized when to_synapse_error is called and the original error body did not have an error property:

errcode = j.pop("errcode", Codes.UNKNOWN)
errmsg = j.pop("error", self.msg)

Reported from @richvdh with the stack:

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/synapse/synapse/synapse/http/server.py", line 232, in _async_render_wrapper
    self._send_response(request, code, response)
  File "/usr/lib/python3.5/contextlib.py", line 77, in __exit__
    self.gen.throw(type, value, traceback)
  File "/opt/synapse/synapse/synapse/logging/opentracing.py", line 815, in trace_servlet
    yield
  File "/opt/synapse/synapse/synapse/http/server.py", line 228, in _async_render_wrapper
    callback_return = await self._async_render(request)
  File "/opt/synapse/synapse/synapse/http/server.py", line 254, in _async_render
    callback_return = await raw_callback_return
  File "/opt/synapse/synapse/synapse/rest/media/v1/thumbnail_resource.py", line 70, in _async_render_GET
    request, server_name, media_id, width, height, method, m_type
  File "/opt/synapse/synapse/synapse/rest/media/v1/thumbnail_resource.py", line 246, in _respond_remote_thumbnail
    media_info = await self.media_repo.get_remote_media_info(server_name, media_id)
  File "/opt/synapse/synapse/synapse/rest/media/v1/media_repository.py", line 278, in get_remote_media_info
    server_name, media_id
  File "/usr/lib/python3.5/contextlib.py", line 77, in __exit__
    self.gen.throw(type, value, traceback)
  File "/opt/synapse/synapse/synapse/util/async_helpers.py", line 263, in _ctx_manager
    yield
  File "/opt/synapse/synapse/synapse/rest/media/v1/media_repository.py", line 278, in get_remote_media_info
    server_name, media_id
  File "/opt/synapse/synapse/synapse/rest/media/v1/media_repository.py", line 326, in _get_remote_media_impl
    media_info = await self._download_remote_file(server_name, media_id, file_id)
  File "/opt/synapse/synapse/synapse/rest/media/v1/media_repository.py", line 401, in _download_remote_file
    await finish()
  File "/usr/lib/python3.5/contextlib.py", line 77, in __exit__
    self.gen.throw(type, value, traceback)
  File "/opt/synapse/synapse/synapse/rest/media/v1/media_storage.py", line 128, in store_into_file
    yield f, fname, finish
  File "/opt/synapse/synapse/synapse/rest/media/v1/media_repository.py", line 384, in _download_remote_file
    raise e.to_synapse_error()
synapse.api.errors.ProxiedRequestError: 404: b'Not Found'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/synapse/synapse/synapse/http/site.py", line 162, in processing
    yield
  File "/opt/synapse/synapse/synapse/http/server.py", line 168, in wrapped_async_request_handler
    await h(self, request)
  File "/opt/synapse/synapse/synapse/http/server.py", line 238, in _async_render_wrapper
    self._send_error_response(f, request)
  File "/opt/synapse/synapse/synapse/http/server.py", line 300, in _send_error_response
    return_json_error(f, request)
  File "/opt/synapse/synapse/synapse/http/server.py", line 101, in return_json_error
    pretty_print=_request_user_agent_is_curl(request),
  File "/opt/synapse/synapse/synapse/http/server.py", line 533, in respond_with_json
    json_bytes = encode_canonical_json(json_object)
  File "/opt/synapse/env3/lib/python3.5/site-packages/canonicaljson.py", line 159, in encode_canonical_json
    s = _canonical_encoder.encode(json_object)
  File "/usr/lib/python3.5/json/encoder.py", line 198, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/lib/python3.5/json/encoder.py", line 256, in iterencode
    return _iterencode(o, 0)
  File "/opt/synapse/env3/lib/python3.5/site-packages/canonicaljson.py", line 32, in _default
    obj.__class__.__name__)
TypeError: Object of type bytes is not JSON serializable

@clokep clokep force-pushed the clokep/http-response-exc branch from e68a0fa to c697eb5 Compare July 29, 2020 14:06
@clokep clokep requested a review from a team July 29, 2020 14:11
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm

@clokep clokep merged commit a53e016 into develop Jul 29, 2020
@clokep clokep deleted the clokep/http-response-exc branch July 29, 2020 17:56
anoadragon453 added a commit that referenced this pull request Jul 30, 2020
…bership_join_count

* 'develop' of github.com:matrix-org/synapse:
  Update workers docs (#7990)
  Fix invite rejection when we have no forward-extremeties (#7980)
  Fix typo in docs/workers.md (#7992)
  Convert federation client to async/await. (#7975)
  Convert appservice to async. (#7973)
  Convert some of the data store to async. (#7976)
  Fix formatting of changelog and upgrade notes
  Ensure that remove_pusher is always async (#7981)
  Add deprecation warnings
  1.18.0
  Update worker docs with recent enhancements  (#7969)
  Ensure the msg property of HttpResponseException is a string. (#7979)
  Remove from the event_relations table when purging historical events. (#7978)
  Add additional logging for SAML sessions. (#7971)
  Add MSC reference to changelog for #7736
  Re-implement unread counts (#7736)
  Various improvements to the docs (#7899)
reivilibre added a commit that referenced this pull request Aug 13, 2020
Synapse 1.19.0rc1 (2020-08-13)
==============================

Removal warning
---------------

As outlined in the [previous release](https://github.com/matrix-org/synapse/releases/tag/v1.18.0), we are no longer publishing Docker images with the `-py3` tag suffix. On top of that, we have also removed the `latest-py3` tag. Please see [the announcement in the upgrade notes for 1.18.0](https://github.com/matrix-org/synapse/blob/develop/UPGRADE.rst#upgrading-to-v1180).

Features
--------

- Add option to allow server admins to join rooms which fail complexity checks. Contributed by @lugino-emeritus. ([\#7902](#7902))
- Add an option to purge room or not with delete room admin endpoint (`POST /_synapse/admin/v1/rooms/<room_id>/delete`). Contributed by @dklimpel. ([\#7964](#7964))
- Add rate limiting to users joining rooms. ([\#8008](#8008))
- Add a `/health` endpoint to every configured HTTP listener that can be used as a health check endpoint by load balancers. ([\#8048](#8048))
- Allow login to be blocked based on the values of SAML attributes. ([\#8052](#8052))
- Allow guest access to the `GET /_matrix/client/r0/rooms/{room_id}/members` endpoint, according to MSC2689. Contributed by Awesome Technologies Innovationslabor GmbH. ([\#7314](#7314))

Bugfixes
--------

- Fix a bug introduced in Synapse v1.7.2 which caused inaccurate membership counts in the room directory. ([\#7977](#7977))
- Fix a long standing bug: 'Duplicate key value violates unique constraint "event_relations_id"' when message retention is configured. ([\#7978](#7978))
- Fix "no create event in auth events" when trying to reject invitation after inviter leaves. Bug introduced in Synapse v1.10.0. ([\#7980](#7980))
- Fix various comments and minor discrepencies in server notices code. ([\#7996](#7996))
- Fix a long standing bug where HTTP HEAD requests resulted in a 400 error. ([\#7999](#7999))
- Fix a long-standing bug which caused two copies of some log lines to be written when synctl was used along with a MemoryHandler logger. ([\#8011](#8011), [\#8012](#8012))

Updates to the Docker image
---------------------------

- We no longer publish Docker images with the `-py3` tag suffix, as [announced in the upgrade notes](https://github.com/matrix-org/synapse/blob/develop/UPGRADE.rst#upgrading-to-v1180). ([\#8056](#8056))

Improved Documentation
----------------------

- Document how to set up a client .well-known file and fix several pieces of outdated documentation. ([\#7899](#7899))
- Improve workers docs. ([\#7990](#7990), [\#8000](#8000))
- Fix typo in `docs/workers.md`. ([\#7992](#7992))
- Add documentation for how to undo a room shutdown. ([\#7998](#7998), [\#8010](#8010))

Internal Changes
----------------

- Reduce the amount of whitespace in JSON stored and sent in responses. Contributed by David Vo. ([\#7372](#7372))
- Switch to the JSON implementation from the standard library and bump the minimum version of the canonicaljson library to 1.2.0. ([\#7936](#7936), [\#7979](#7979))
- Convert various parts of the codebase to async/await. ([\#7947](#7947), [\#7948](#7948), [\#7949](#7949), [\#7951](#7951), [\#7963](#7963), [\#7973](#7973), [\#7975](#7975), [\#7976](#7976), [\#7981](#7981), [\#7987](#7987), [\#7989](#7989), [\#8003](#8003), [\#8014](#8014), [\#8016](#8016), [\#8027](#8027), [\#8031](#8031), [\#8032](#8032), [\#8035](#8035), [\#8042](#8042), [\#8044](#8044), [\#8045](#8045), [\#8061](#8061), [\#8062](#8062), [\#8063](#8063), [\#8066](#8066), [\#8069](#8069), [\#8070](#8070))
- Move some database-related log lines from the default logger to the database/transaction loggers. ([\#7952](#7952))
- Add a script to detect source code files using non-unix line terminators. ([\#7965](#7965), [\#7970](#7970))
- Log the SAML session ID during creation. ([\#7971](#7971))
- Implement new experimental push rules for some users. ([\#7997](#7997))
- Remove redundant and unreliable signature check for v1 Identity Service lookup responses. ([\#8001](#8001))
- Improve the performance of the register endpoint. ([\#8009](#8009))
- Reduce less useful output in the newsfragment CI step. Add a link to the changelog section of the contributing guide on error. ([\#8024](#8024))
- Rename storage layer objects to be more sensible. ([\#8033](#8033))
- Change the default log config to reduce disk I/O and storage for new servers. ([\#8040](#8040))
- Add an assertion on `prev_events` in `create_new_client_event`. ([\#8041](#8041))
- Add a comment to `ServerContextFactory` about the use of `SSLv23_METHOD`. ([\#8043](#8043))
- Log `OPTIONS` requests at `DEBUG` rather than `INFO` level to reduce amount logged at `INFO`. ([\#8049](#8049))
- Reduce amount of outbound request logging at `INFO` level. ([\#8050](#8050))
- It is no longer necessary to explicitly define `filters` in the logging configuration. (Continuing to do so is redundant but harmless.) ([\#8051](#8051))
- Add and improve type hints. ([\#8058](#8058), [\#8064](#8064), [\#8060](#8060), [\#8067](#8067))
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit '3950ae51e':
  Ensure that remove_pusher is always async (#7981)
  Ensure the msg property of HttpResponseException is a string. (#7979)
  Remove from the event_relations table when purging historical events. (#7978)
  Add additional logging for SAML sessions. (#7971)
  Add MSC reference to changelog for #7736
  Re-implement unread counts (#7736)
  Various improvements to the docs (#7899)
  Convert storage layer to async/await. (#7963)
  Add an option to disable purge in delete room admin API (#7964)
  Move some log lines from default logger to sql/transaction loggers (#7952)
  Use the JSON module from the std library instead of simplejson. (#7936)
  Fix exit code for `check_line_terminators.sh` (#7970)
  Option to allow server admins to join complex rooms (#7902)
  Fix typo in metrics docs (#7966)
  Add script for finding files with unix line terminators (#7965)
  Convert the remaining media repo code to async / await. (#7947)
  Convert a synapse.events to async/await. (#7949)
  Convert groups and visibility code to async / await. (#7951)
  Convert push to async/await. (#7948)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants