Skip to content

Commit

Permalink
Adjust health check logic of wait_for_service function
Browse files Browse the repository at this point in the history
There are a few potential issues of note with the `wait_for_service` function. This function
is intended to check if a service is ready by ensuring its health check passes before
interacting with it.

* The first potential issue is the URL construction.  The string `/healthcheck` is appended
to the end of the given URL. This assumes that the URL given does not have a path at all.
* The second potential issue has to do with how we check for a "success".  The success is
determined by receiving any response back from the server.  This means that if the server
returns an error code, such as `404`, the health check would still pass. Normally health
checks must ensure that a 200 status code is returned.

To fix these potential issues we addressed them directly.

First he URL construction was modified to use the `Url::join` method, as it is aware of
relative and absolute path references.

Second the reqwest call was re-written to ensure that the response retrieved is an Ok.
This didn't have a direct chagne to the logic, but helped with the readability of
the next portion, and it ensures that the `response` variable persists beyond a single
closure.

Third the response's status code is checked with the
`http::status::StatusCode::is_success` method.

Finally, some comments were added to provide a simple explanation of what the
`wait_for_service` function is doing, and some behavior implications.

Beyond these changes, no other behavior modifications have been made.  This means that
the health check still checks to see of the `Response` is able to retrieve a textual
representation of the response.
  • Loading branch information
Ayiga committed Jan 10, 2025
1 parent ea449b8 commit beb58f1
Showing 1 changed file with 30 additions and 9 deletions.
39 changes: 30 additions & 9 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,20 +237,41 @@ pub async fn get_builder_address(url: Url) -> Address {
panic!("Error: Failed to retrieve address from builder!");
}

/// [wait_for_service] will check to see if a service, identified by the given
/// Url, is available, by checking it's health check endpoint. If the health
/// check does not any time before the timeout, then the service will return
/// an [Err] with the relevant error.
///
/// > Note: This function only waits for a single health check pass before
/// > returning an [Ok] result.
async fn wait_for_service(url: Url, interval: u64, timeout_duration: u64) -> Result<String> {
// utilize the correct path for the health check
let Ok(url) = url.join("/healthcheck") else {
return Err(anyhow!("Wait for service, could not join url: {}", url));
};

timeout(Duration::from_secs(timeout_duration), async {
loop {
if let Ok(body) = reqwest::get(format!("{url}/healthcheck")).await {
return body.text().await.map_err(|e| {
anyhow!(
"Wait for service, could not decode response: ({}) {}",
url,
e
)
});
} else {
// Ensure that we get a response from the server
let Ok(response) = reqwest::get(url.clone()).await else {
sleep(Duration::from_millis(interval)).await;
continue;
};

// Check the status code of the response
if !response.status().is_success() {
// The server did not return a success
sleep(Duration::from_millis(interval)).await;
continue;
}

return response.text().await.map_err(|e| {
anyhow!(
"Wait for service, could not decode response: ({}) {}",
url,
e
)
});
}
})
.await
Expand Down

0 comments on commit beb58f1

Please sign in to comment.