Refactor pub http retry behavior

[sigurdm]

• timeouts on dns lookups (if possible)

• timeouts on http requests
  => timeout 30 seconds on request-> response headers

• timeouts on body download

Length limits:

• Stop downloading after content-length is reached.
• Check-sum of google checksum headers when present Validate CRC32c if x-goog-hash header is present #2281
• Sanity length limit on version listings.

Improved error behavior:

• Retry after 429 status code.
• Don't crash on 403: HTTP error 403: Forbidden #3256.
• Retry failure during response-streaming -> we need a fetch function
• Improve handling of "secret" error codes
• On error codes, write the url it came from, and possible (part of) the body

[nehalvpatel]

I've captured the current state of the current retry logic so we can reference it while refactoring.

Caveat: the retry() method with the CRC32C validation is not merged as of the writing of this comment.

sequenceDiagram
    _download ->> retry(): <invoke>
    retry() ->> _AuthenticatedClient: make new request and .send()
    _AuthenticatedClient ->> _ThrottleClient: insert auth headers and .send()
    Note over _AuthenticatedClient, _ThrottleClient: ⏳ _ThrottleClient can (re)throw PubHttpException<br/>_AuthenticatedClient catches and if status code is 403,<br/>throws AuthenticationException 🚨
    _ThrottleClient ->> _ThrowingClient: gently .send()
    _ThrowingClient ->> RetryClient: .send()
    Note over _ThrowingClient,RetryClient: ⏳ RetryClient can (re)throw SocketException<br/>_ThrowingClient catches and calls fail() 🛑 on specific error codes.<br/>Otherwise, it rethrows 🚨
    RetryClient ->> _PubHttpClient: .send() cloned, finalized request
    Note over RetryClient,_PubHttpClient: ⏳ _PubHttpClient can (re)throw IOException<br/><br/>RetryClient catches and calls .send() again if attempts left
    _PubHttpClient ->> http.BaseClient: insert Pub headers and .send()
    http.BaseClient ->> _PubHttpClient: Sucessful response or<br/>bad response
    _PubHttpClient ->> RetryClient: Sucessful response or<br/>bad response
    RetryClient ->> RetryClient: Checks for status codes 500, 502, 503, and 504<br/>and calls .send() again if attempts left
    RetryClient ->> _ThrowingClient: Sucessful response or<br/>bad response after retry limit
    _ThrowingClient ->> _ThrowingClient: Checks for status code `< 400`, `== 401`, or `== 400 && isTokenRequest`. If match, returns response ⬅️<br/>Else, checks for status code 406 and 'Accept' request header. If match, calls fail() 🛑<br/>Else, checks for status code 500 and request url host. If match, calls fail() 🛑<br/>Finally, throws PubHttpException before fall through 🚨
    _ThrowingClient ->> _ThrottleClient: Successful response or<br/>bad auth response
    _ThrottleClient ->> _AuthenticatedClient: Successful response or<br/>bad auth response
    _AuthenticatedClient ->> _AuthenticatedClient: If status code is 401,<br/>throws AuthenticationException 🚨
    _AuthenticatedClient ->> retry(): Successful response
    retry() ->> retry(): Validates CRC32C if applicable<br/>Catches PackageIntegrityException and retry()s if attempts left
    retry() ->> _download: Validated response

Loading

[nehalvpatel]

#3590 improves retries and addresses some of these issues:

• Mitigated intermittent connection/low-level networking errors with retries. We no longer check for OS-specific error codes.

  • pub/lib/src/http.dart

    Lines 259 to 294 in a73598b

    	/// Whether [e] is one of a few HTTP-related exceptions that subclass
    	/// [IOException]. Can be used if your try-catch block contains various
    	/// operations in addition to HTTP calls and so a [IOException] instance check
    	/// would be too coarse.
    	bool isHttpIOException(Object e) {
    	return e is HttpException ||
    	e is TlsException ||
    	e is SocketException ||
    	e is WebSocketException;
    	}
    	/// Program-wide limiter for concurrent network requests.
    	final _httpPool = Pool(16);
    	/// Runs the provided function [fn] and returns the response.
    	///
    	/// If there is an HTTP-related exception, an intermittent HTTP error response,
    	/// or an async timeout, [fn] is run repeatedly until there is a successful
    	/// response or at most seven total attempts have been made. If all attempts
    	/// fail, the final exception is re-thrown.
    	///
    	/// Each attempt is run within a [Pool] configured with 16 maximum resources.
    	Future<T> retryForHttp<T>(String operation, FutureOr<T> Function() fn) async {
    	return await retry(
    	() async => await _httpPool.withResource(() async => await fn()),
    	retryIf: (e) async =>
    	(e is PubHttpException && e.isIntermittent) ||
    	e is TimeoutException ||
    	isHttpIOException(e),
    	onRetry: (exception, attemptNumber) async =>
    	log.io('Attempt #$attemptNumber for $operation'),
    	maxAttempts: math.max(
    	1, // Having less than 1 attempt doesn't make sense.
    	int.tryParse(Platform.environment['PUB_MAX_HTTP_RETRIES'] ?? '') ?? 7,
    	));
    	}

• Fixed case where client could crash from a 403 response. Consolidated logic for validating the response status code.

  • pub/lib/src/http.dart

    Lines 306 to 335 in a73598b

    	/// Throws [PubHttpResponseException], calls [fail], or does nothing depending
    	/// on the status code.
    	///
    	/// If the code is in the 200 range or if its a 300 range redirect code,
    	/// nothing is done. If the code is 408, 429, or in the 500 range,
    	/// [PubHttpResponseException] is thrown with "isIntermittent" set to `true`.
    	/// Otherwise, [PubHttpResponseException] is thrown with "isIntermittent" set
    	/// to `false`.
    	void throwIfNotOk() {
    	if (statusCode >= 200 && statusCode <= 299) {
    	return;
    	} else if (_redirectStatusCodes.contains(statusCode)) {
    	return;
    	} else if (statusCode == HttpStatus.notAcceptable &&
    	request?.headers['Accept'] == pubApiHeaders['Accept']) {
    	fail('Pub ${sdk.version} is incompatible with the current version of '
    	'${request?.url.host}.\n'
    	'Upgrade pub to the latest version and try again.');
    	} else if (statusCode >= 500 ||
    	statusCode == HttpStatus.requestTimeout ||
    	statusCode == HttpStatus.tooManyRequests) {
    	// Throw if the response indicates a server error or an intermittent
    	// client error, but mark it as intermittent so it can be retried.
    	throw PubHttpResponseException(this, isIntermittent: true);
    	} else {
    	// Throw for all other status codes.
    	throw PubHttpResponseException(this);
    	}
    	}
    	}

• Added retries to OAuth2 login and publishing process