Skip to content

refactoring spike: simplify ProxyCommand.timeout and ProxyRequest.timeout #3106

@cfm

Description

@cfm

3a1d301 has me convinced that we set separately both a process-level (ProxyCommand) timeout and a request-level (ProxyRequest) timeout, with some impedance mismatch that we then have to reconcile:

  1. ProxyCommand.timeout is in seconds because that's what securedrop-proxy expects:
    timeout: u64, // sec

    .timeout(Duration::from_secs(incoming_request.timeout));
  2. ProxyRequest.timeout is in milliseconds because that's the standard unit in JavaScript for setting timers (intervals).
  3. We need ProxyCommand.timeout >= ProxyRequest.timeout, or else we see cases like app: initial sync of a large index can time out #3048 where a very live proxy command has been terminated rather than timing out.
  4. We construct a ProxyCommand for each ProxyRequest anyway.

3a1d301 introduces some nominal typing to address (1) and (2), and I think it helps enforce (3). But (4) makes me think that we can simplify these paths so that the caller sets a single request-level timeout of $t$ seconds, and the command automatically inherits a process-level timeout of $t+1$ seconds.

Metadata

Metadata

Assignees

No one assigned

    Labels

    RefactoringThis PR is a strict refactoring.

    Type

    Projects

    Status

    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions