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

Downgrade warning on client disconnect to INFO #7928

Merged
merged 3 commits into from
Jul 24, 2020

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Jul 21, 2020

Clients disconnecting before we finish processing the request happens from time
to time. We don't need to yell about it.

Clients disconnecting before we finish processing the request happens from time
to time. We don't need to yell about it.
@richvdh richvdh requested a review from a team July 21, 2020 23:37
@richvdh richvdh changed the title Downgrade warning on client disconnect. Downgrade warning on client disconnect to INFO Jul 21, 2020
@clokep
Copy link
Member

clokep commented Jul 22, 2020

Looks like the tests are unhappy with this change though.

@richvdh
Copy link
Member Author

richvdh commented Jul 23, 2020

yeah, I've just ripped out the offending test: I don't really think we need to test that we're logging something here. Would appreciate a sanity-check though.

@richvdh richvdh requested a review from clokep July 23, 2020 15:56
@clokep
Copy link
Member

clokep commented Jul 23, 2020

Looks like that test was testing whether the access token is redacted, which seems useful? I'm guessing this failed because tests don't log at INFO by default?

I don't have a strong opinion either way about that test though.

@richvdh
Copy link
Member Author

richvdh commented Jul 23, 2020

Looks like that test was testing whether the access token is redacted, which seems useful? I'm guessing this failed because tests don't log at INFO by default? I'm guessing this failed because tests don't log at INFO by default?

well it would be useful, if we were logging the request URI at all. It fails because the new log message doesn't inclued the request URI.

Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Looks good if CI passes. 👍

@richvdh richvdh merged commit 1ec688b into develop Jul 24, 2020
@richvdh richvdh deleted the rav/remove_scary_warning branch July 24, 2020 08:55
anoadragon453 added a commit that referenced this pull request Jul 28, 2020
…rove_test_times

* 'develop' of github.com:matrix-org/synapse: (148 commits)
  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)
  update changelog
  1.18.0rc1
  Fix error reporting when using `opentracing.trace` (#7961)
  Fix typing replication not being handled on master (#7959)
  Remove hacky error handling for inlineDeferreds. (#7950)
  Convert tests/rest/admin/test_room.py to unix file endings (#7953)
  Support oEmbed for media previews. (#7920)
  Convert state resolution to async/await (#7942)
  Fix up types and comments that refer to Deferreds. (#7945)
  Do not convert async functions to Deferreds in the interactive_auth_handler (#7944)
  Convert more of the media code to async/await (#7873)
  Return an empty body for OPTIONS requests. (#7886)
  Downgrade warning on client disconnect to INFO (#7928)
  Convert presence handler helpers to async/await. (#7939)
  Update the auth providers to be async. (#7935)
  ...
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'f88c48f3b':
  1.18.0rc1
  Fix error reporting when using `opentracing.trace` (#7961)
  Fix typing replication not being handled on master (#7959)
  Remove hacky error handling for inlineDeferreds. (#7950)
  Convert tests/rest/admin/test_room.py to unix file endings (#7953)
  Support oEmbed for media previews. (#7920)
  Convert state resolution to async/await (#7942)
  Fix up types and comments that refer to Deferreds. (#7945)
  Do not convert async functions to Deferreds in the interactive_auth_handler (#7944)
  Convert more of the media code to async/await (#7873)
  Return an empty body for OPTIONS requests. (#7886)
  Downgrade warning on client disconnect to INFO (#7928)
  Convert presence handler helpers to async/await. (#7939)
  Update the auth providers to be async. (#7935)
  Put a cache on `/state_ids` (#7931)
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