Skip to content
Snippets Groups Projects
  • Sean Quah's avatar
    14b8c047
    Prevent logging context going missing on federation request timeout (#10810) · 14b8c047
    Sean Quah authored
    In `MatrixFederationHttpClient._send_request()`, we make a HTTP request
    using an `Agent`, wrap that request in a timeout and await the resulting
    `Deferred`. On its own, the `Agent` performing the HTTP request
    correctly stashes and restores the logging context while waiting.
    The addition of the timeout introduces a path where the logging context
    is not restored when execution resumes.
    
    To address this, we wrap the timeout `Deferred` in a
    `make_deferred_yieldable()` to stash the logging context and restore it
    on completion of the `await`. However this is not sufficient, since by
    the time we construct the timeout `Deferred`, the `Agent` has already
    stashed and cleared the logging context when using
    `make_deferred_yieldable()` to produce its `Deferred` for the request.
    
    Hence, we wrap the `Agent` request in a `run_in_background()` to "fork"
    and preserve the logging context so that we can stash and restore it
    when `await`ing the timeout `Deferred`.
    
    This approach is similar to the one used with `defer.gatherResults`.
    
    Note that the code is still not fully correct. When a timeout occurs,
    the request remains running in the background (existing behavior which
    is nothing to do with the new call to `run_in_background`) and may
    re-start the logging context after it has finished.
    14b8c047
    History
    Prevent logging context going missing on federation request timeout (#10810)
    Sean Quah authored
    In `MatrixFederationHttpClient._send_request()`, we make a HTTP request
    using an `Agent`, wrap that request in a timeout and await the resulting
    `Deferred`. On its own, the `Agent` performing the HTTP request
    correctly stashes and restores the logging context while waiting.
    The addition of the timeout introduces a path where the logging context
    is not restored when execution resumes.
    
    To address this, we wrap the timeout `Deferred` in a
    `make_deferred_yieldable()` to stash the logging context and restore it
    on completion of the `await`. However this is not sufficient, since by
    the time we construct the timeout `Deferred`, the `Agent` has already
    stashed and cleared the logging context when using
    `make_deferred_yieldable()` to produce its `Deferred` for the request.
    
    Hence, we wrap the `Agent` request in a `run_in_background()` to "fork"
    and preserve the logging context so that we can stash and restore it
    when `await`ing the timeout `Deferred`.
    
    This approach is similar to the one used with `defer.gatherResults`.
    
    Note that the code is still not fully correct. When a timeout occurs,
    the request remains running in the background (existing behavior which
    is nothing to do with the new call to `run_in_background`) and may
    re-start the logging context after it has finished.