Skip to content

feat(uptime): remove built-in 2xx status code check #484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
evanpurkhiser wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(uptime): remove built-in 2xx status code check #484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2741f7b
feat(uptime): remove built-in 2xx status code check
evanpurkhiser Feb 20, 2026
d8dd2ac
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Feb 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/checker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn make_trace_header(config: &CheckConfig, trace_id: &Uuid, span_id: SpanId) ->
/// which the check is being made.
pub trait Checker: Send + Sync {
/// Makes a request to a url to determine whether it is up.
/// Up is defined as returning a 2xx within a specific timeframe.
/// Up is defined as responding within a specific timeframe, optionally validated by assertions.
fn check_url(
&self,
check: &ScheduledCheck,
Expand Down
36 changes: 21 additions & 15 deletions src/checker/reqwest_checker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,18 +334,11 @@ fn to_check_result(
max_assertion_ops,
region,
)
} else if r.status().is_success() {
Check::success()
} else {
Check::code_failure(r.status())
Check::success()
}
} else {
// TODO: rust 2024 allows let-chaining, so the enclosing if-statement can be
// folded into the the if let
match r.status().is_success() {
true => Check::success(),
false => Check::code_failure(r.status()),
}
Check::success()
}
Comment on lines 334 to 342
Copy link
Copy Markdown

@sentry sentry Bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Uptime checks without an explicit assertion now treat non-2xx HTTP responses as successful, which can lead to silent failures for existing checks.
Severity: HIGH

Suggested Fix

To prevent silent failures for existing checks, consider adding a default status code assertion when a CheckConfig is loaded without one. For example, if check.get_config().assertion is None, automatically apply an assertion that validates the status code is within the 2xx range. This would maintain backward compatibility while still allowing users to override it with explicit assertions.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/checker/reqwest_checker.rs#L334-L342

Potential issue: The pull request removes the implicit validation of 2xx HTTP status
codes for uptime checks. In the new implementation, if a `CheckConfig` does not have an
explicit `assertion` configured, any HTTP response, including those with 4xx or 5xx
status codes, will be considered a success. This is a breaking change from the previous
behavior where non-2xx responses were automatically treated as failures. Existing uptime
checks that rely on this implicit validation will now silently report success even when
the service is down, potentially leading to undetected outages.

Did we get this right? 👍 / 👎 to inform future reviews.

}
Err(e) => Check::other_failure(e.into()),
Expand Down Expand Up @@ -510,7 +503,7 @@ fn to_errored_request_infos(

impl Checker for ReqwestChecker {
/// Makes a request to a url to determine whether it is up.
/// Up is defined as returning a 2xx within a specific timeframe, along with executing
/// Up is defined as responding within a specific timeframe, along with passing
/// an optional user-defined assert that is specified by the user which can use
/// the result json, status code, and response header values.
#[tracing::instrument]
Expand Down Expand Up @@ -889,7 +882,7 @@ mod tests {
}

#[tokio::test]
async fn test_simple_400() {
async fn test_simple_400_no_assertion() {
let server = MockServer::start();
let checker = ReqwestChecker::new_internal(
Options {
Expand All @@ -916,14 +909,13 @@ mod tests {
let check = ScheduledCheck::new_for_test(tick, config);
let result = checker.check_url(&check, "us-west").await;

assert_eq!(result.status, CheckStatus::Failure);
// Without an assertion, a non-2xx response is still considered a success.
// Callers are expected to configure a status code assertion to validate the response.
assert_eq!(result.status, CheckStatus::Success);
assert_eq!(
result.request_info.and_then(|i| i.http_status_code),
Some(400)
);
let reason = result.status_reason.unwrap();
assert_eq!(reason.status_type, CheckStatusReasonType::Failure);
assert_eq!(reason.description, "Got non 2xx status: 400 Bad Request");

head_mock.assert();
}
Expand Down Expand Up @@ -953,6 +945,13 @@ mod tests {

let config = CheckConfig {
url: server.url("/error").to_string(),
assertion: crate::assertions::Assertion {
root: crate::assertions::Op::StatusCodeCheck {
value: 500,
operator: crate::assertions::Comparison::NotEqual,
},
}
.into(),
..Default::default()
};

Expand Down Expand Up @@ -1007,6 +1006,13 @@ mod tests {

let config = CheckConfig {
url: server.url("/error").to_string(),
assertion: crate::assertions::Assertion {
root: crate::assertions::Op::StatusCodeCheck {
value: 500,
operator: crate::assertions::Comparison::NotEqual,
},
}
.into(),
..Default::default()
};

Expand Down
13 changes: 0 additions & 13 deletions src/types/result.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::assertions::compiled;
use crate::assertions::compiled::EvalPath;
use crate::assertions::Assertion;
use chrono::{DateTime, TimeDelta, Utc};
use http::StatusCode;
use hyper::rt::ConnectionStats;
use hyper::stats::AbsoluteDuration;
use hyper::stats::RequestStats;
Expand Down Expand Up @@ -414,18 +413,6 @@ impl Check {
}
}

pub fn code_failure(status: StatusCode) -> Self {
Self {
result: CheckStatus::Failure,
reason: Some(CheckStatusReason {
status_type: CheckStatusReasonType::Failure,
description: format!("Got non 2xx status: {status}"),
details: None,
}),
assert_path: None,
}
}

pub fn other_failure(reason: CheckStatusReason) -> Self {
Self {
result: CheckStatus::Failure,
Expand Down
Loading