fix: auto retry once for connection closed

[inventvenkat]

Type of Change

• Bugfix
• New feature
• Enhancement
• Refactoring
• Dependency updates
• Documentation
• CI/CD

Description

Hyper crate has a bug which allows few calls to connect with a dead connections and fails to send request to the server. This bug of hyper can be tracked with below link.
Error: IncompleteMessage: connection closed before message completed - Link

Temporary fix: Retry connection with the server by sending the request again when call fails with this issue.

Permanent fix would be, whenever reqwest crate release a stable version with hyper's 1.0 version

Another Link
This is just due to the racy nature of networking. hyper has a connection pool of idle connections, and it selected one to send your request. Most of the time, hyper will receive the server’s FIN and drop the dead connection from its pool. But occasionally, a connection will be selected from the pool and written to at the same time the server is deciding to close the connection. Since hyper already wrote some of the request, it can’t really retry it automatically on a new connection, since the server may have acted already

Additional Changes

• This PR modifies the API contract
• This PR modifies the database schema
• This PR modifies application configuration/environment variables

Motivation and Context

Few external api calls fails due to this connection close

How did you test it?

Running the test case collections

Checklist

• I formatted the code cargo +nightly fmt --all
• I addressed lints thrown by cargo clippy
• I reviewed the submitted code
• I added unit tests for my changes where possible
• I added a CHANGELOG entry if applicable

[NishantJoshi00]

Other than that, looks good to me

[NishantJoshi00]

Suggested change

	Err(error) => {
	if error.current_context() == &errors::ApiClientError::ConnectionClosed {
	metrics::AUTO_RETRY_CONNECTION_CLOSED.add(&metrics::CONTEXT, 1, &[]);
	match cloned_send_request {
	Some(cloned_request) => {
	metrics_request::record_operation_time(
	cloned_request,
	&metrics::EXTERNAL_REQUEST_TIME,
	&[metrics_tag],
	)
	.await
	}
	None => Err(error),
	}
	} else {
	Err(error)
	}
	}
	Err(error) if error.current_context() == &errors::ApiClientError::ConnectionClosed => {
	metrics::AUTO_RETRY_CONNECTION_CLOSED.add(&metrics::CONTEXT, 1, &[]);
	match cloned_send_request {
	Some(cloned_request) => {
	metrics_request::record_operation_time(
	cloned_request,
	&metrics::EXTERNAL_REQUEST_TIME,
	&[metrics_tag],
	)
	.await
	}
	None => Err(error),
	}
	},
	err @ Err(_) => err

[inventvenkat]

@jarnura , this Error: IncompleteMessage: connection closed before message completed is the error message by hyper, when the connection gets closed before the message could complete from the client side. Which infers that we are not able to send the message to the server.
https://docs.rs/hyper/0.14.28/hyper/struct.Error.html#method.is_incomplete_message

[jarnura]

Here we are retrying for all connection closed cases, but for non-idempotent request this will cause business related errors and audit for this is complex.

[inventvenkat]

#3426 (comment)

[Narayanbhat166]

@inventvenkat instead of doing this, what if we do not have any idle_pools, AFAIK the error is caused mainly because of the idle pools. Would we have any significant performance hit without idle pools?

[inventvenkat]

@inventvenkat instead of doing this, what if we do not have any idle_pools, AFAIK the error is caused mainly because of the idle pools. Would we have any significant performance hit without idle pools?

pool_idle_timeout disabling timeout didn't help this case.