HttpPoolManager._executeWithRetry - ClientException/SocketException crash

[aaravgarg]

Summary

• Fix fatal crash in HttpPoolManager._executeWithRetry where network errors (SocketException, TimeoutException, ClientException, HandshakeException) are thrown after retry exhaustion, crashing the app
• Return a synthetic 503 response for recoverable network errors instead of propagating exceptions
• Add HandshakeException to the retry catch list for SSL/TLS errors

Crash Stats

• Events: 6,892
• Users affected: 848
• Crashlytics: View in Console

Test plan

• Verify HTTP requests still work normally with good connectivity
• Test behavior with airplane mode / no connectivity (should get 503 response instead of crash)
• Verify retry logic still works for server 5xx errors

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request aims to fix a crash caused by unhandled network exceptions in HttpPoolManager._executeWithRetry by catching them and returning a synthetic 503 error response. The changes in _executeWithRetry and the addition of HandshakeException to the list of caught exceptions are well-implemented. The modifications to sendStreaming are incomplete and inconsistent with the PR's goal, as they only log and rethrow SocketException, which does not prevent a crash and ignores other network errors. The provided suggestion makes the error handling in sendStreaming robust and consistent with the rest of the changes.

[gemini-code-assist]

The change in sendStreaming is inconsistent with the PR's goal of preventing crashes from network errors. It only handles SocketException and rethrows it, which means the caller will still crash if the exception is not handled. Furthermore, other potential network exceptions like TimeoutException, http.ClientException, and HandshakeException are not caught at all.

To align with the robust error handling in _executeWithRetry, sendStreaming should also catch all relevant network exceptions and return a synthetic StreamedResponse instead of rethrowing. This will prevent crashes for callers of this method.

Suggested change

	}) async {
	try {
	return await _client.send(request).timeout(timeout);
	} on SocketException catch (e) {
	Logger.debug('HttpPoolManager sendStreaming SocketException: $e');
	rethrow;
	}
	}
	}) async {
	try {
	return await _client.send(request).timeout(timeout);
	} on Exception catch (e) {
	if (e is SocketException ||
	e is TimeoutException ||
	e is http.ClientException ||
	e is HandshakeException) {
	Logger.debug('HttpPoolManager sendStreaming network error: $e');
	return http.StreamedResponse(Stream<List<int>>.empty(), 503, reasonPhrase: 'Network error: $e');
	}
	rethrow;
	}
	}

[beastoin]

@aaravgarg Quick nudge: tests/demo evidence is required before review. Your crash-fix PRs (#4525–#4549) all have good Crashlytics data and test plans, but the test checkboxes are unchecked. Please add test output or a short screenshot/video showing the fix works, then we can proceed with review. This applies to all PRs in the batch. Thanks!

---

by AI for @beastoin