mirror of
https://github.com/rzuasti/oott.git
synced 2026-07-08 19:21:54 +02:00
Give every API operation a unique operationId and surface scanner errors
Scanner status handlers all derived operationId "status" from their fn name, so Swagger UI's "Try it out" executed the first one (ARP) — the DHCP doc hit /api/arp_scanner/status. Same collision for read/list/ register/unregister. Each path now sets an explicit unique operation_id. main's tokio::join!(...).0 kept only the ARP result and silently dropped the other tasks' errors, so a DHCP scanner that failed to bind port 67 just showed "off" with no log. Each task is now wrapped to log its error. Fixes #5
This commit is contained in:
@@ -124,7 +124,11 @@ mod tests {
|
|||||||
let after_update = list().unwrap();
|
let after_update = list().unwrap();
|
||||||
let rows: Vec<_> = after_update.iter().filter(|t| t.token == token).collect();
|
let rows: Vec<_> = after_update.iter().filter(|t| t.token == token).collect();
|
||||||
assert_eq!(rows.len(), 1, "Re-registering must not create a second row");
|
assert_eq!(rows.len(), 1, "Re-registering must not create a second row");
|
||||||
assert_eq!(rows[0].platform, PushPlatform::Ios, "Platform should update");
|
assert_eq!(
|
||||||
|
rows[0].platform,
|
||||||
|
PushPlatform::Ios,
|
||||||
|
"Platform should update"
|
||||||
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
rows[0].created_on, created_on,
|
rows[0].created_on, created_on,
|
||||||
"created_on must be preserved across an upsert"
|
"created_on must be preserved across an upsert"
|
||||||
|
|||||||
+27
-10
@@ -1,6 +1,7 @@
|
|||||||
use crate::settings::get_settings;
|
use crate::settings::get_settings;
|
||||||
use clap::Parser;
|
use clap::Parser;
|
||||||
use log::{LevelFilter, info};
|
use log::{LevelFilter, error, info};
|
||||||
|
use std::future::Future;
|
||||||
|
|
||||||
mod data;
|
mod data;
|
||||||
mod db;
|
mod db;
|
||||||
@@ -66,15 +67,31 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
|
|||||||
|
|
||||||
// Start the device scanners, web server, retention cleaner, and notification delivery loop in
|
// Start the device scanners, web server, retention cleaner, and notification delivery loop in
|
||||||
// parallel. Notification delivery runs on its own task so a slow Pushover never stalls a scan.
|
// parallel. Notification delivery runs on its own task so a slow Pushover never stalls a scan.
|
||||||
|
// Each task is wrapped so that if it exits with an error (e.g. the DHCP scanner failing to bind
|
||||||
|
// its socket because the port is already in use) the failure is logged rather than silently
|
||||||
|
// swallowed — otherwise a scanner just appears "off" with no explanation.
|
||||||
tokio::join!(
|
tokio::join!(
|
||||||
scanners::arp::scanner::scan(),
|
log_task_errors("ARP scanner", scanners::arp::scanner::scan()),
|
||||||
scanners::mdns::scanner::listen(),
|
log_task_errors("mDNS scanner", scanners::mdns::scanner::listen()),
|
||||||
scanners::ssdp::scanner::listen(),
|
log_task_errors("SSDP scanner", scanners::ssdp::scanner::listen()),
|
||||||
scanners::dhcp::scanner::listen(),
|
log_task_errors("DHCP scanner", scanners::dhcp::scanner::listen()),
|
||||||
scanners::snmp::scanner::scan(),
|
log_task_errors("SNMP scanner", scanners::snmp::scanner::scan()),
|
||||||
web_server::serve(),
|
log_task_errors("web server", web_server::serve()),
|
||||||
retention::run(),
|
retention::run(),
|
||||||
notifications::run_delivery()
|
notifications::run_delivery(),
|
||||||
)
|
);
|
||||||
.0
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Await a long-running task and log its error if it exits with one. Without this the errors of
|
||||||
|
/// every task but the first were dropped by `tokio::join!`, so a scanner that failed to start
|
||||||
|
/// (for example the DHCP scanner being unable to bind port 67) reported no diagnostic at all.
|
||||||
|
async fn log_task_errors<F>(name: &str, task: F)
|
||||||
|
where
|
||||||
|
F: Future<Output = Result<(), Box<dyn std::error::Error>>>,
|
||||||
|
{
|
||||||
|
if let Err(err) = task.await {
|
||||||
|
error!("{name} exited with error: {err}");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -107,7 +107,10 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn platform_parse_is_case_insensitive() {
|
fn platform_parse_is_case_insensitive() {
|
||||||
assert_eq!("ANDROID".parse::<PushPlatform>().unwrap(), PushPlatform::Android);
|
assert_eq!(
|
||||||
|
"ANDROID".parse::<PushPlatform>().unwrap(),
|
||||||
|
PushPlatform::Android
|
||||||
|
);
|
||||||
assert_eq!("iOS".parse::<PushPlatform>().unwrap(), PushPlatform::Ios);
|
assert_eq!("iOS".parse::<PushPlatform>().unwrap(), PushPlatform::Ios);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -62,7 +62,11 @@ async fn deliver(request: DeliveryRequest) {
|
|||||||
// The relay URL defaults to the project-operated relay, so the [notifications.push]
|
// The relay URL defaults to the project-operated relay, so the [notifications.push]
|
||||||
// section is optional. The send call is async (reqwest), so unlike Pushover it is
|
// section is optional. The send call is async (reqwest), so unlike Pushover it is
|
||||||
// awaited directly rather than dispatched to the blocking pool.
|
// awaited directly rather than dispatched to the blocking pool.
|
||||||
let config = get_settings().notifications.push.clone().unwrap_or_default();
|
let config = get_settings()
|
||||||
|
.notifications
|
||||||
|
.push
|
||||||
|
.clone()
|
||||||
|
.unwrap_or_default();
|
||||||
if let Err(err) = push::send(&config, request.title, request.body).await {
|
if let Err(err) = push::send(&config, request.title, request.body).await {
|
||||||
error!("Failed to deliver notification via the push relay: {err}");
|
error!("Failed to deliver notification via the push relay: {err}");
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -77,7 +77,10 @@ pub async fn send(config: &Push, title: String, body: String) -> Result<usize, D
|
|||||||
let parsed: RelayResponse = response.json().await?;
|
let parsed: RelayResponse = response.json().await?;
|
||||||
let dead = dead_tokens(&parsed.results);
|
let dead = dead_tokens(&parsed.results);
|
||||||
if !dead.is_empty() {
|
if !dead.is_empty() {
|
||||||
debug!("Pruning {} dead push token(s) reported by the relay", dead.len());
|
debug!(
|
||||||
|
"Pruning {} dead push token(s) reported by the relay",
|
||||||
|
dead.len()
|
||||||
|
);
|
||||||
db::run_blocking(move || db::push_tokens::delete_many(&dead)).await?;
|
db::run_blocking(move || db::push_tokens::delete_many(&dead)).await?;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -150,7 +153,10 @@ mod tests {
|
|||||||
relay_url: format!("http://{addr}/v1/push"),
|
relay_url: format!("http://{addr}/v1/push"),
|
||||||
};
|
};
|
||||||
let delivered = send(&config, "title".into(), "body".into()).await.unwrap();
|
let delivered = send(&config, "title".into(), "body".into()).await.unwrap();
|
||||||
assert_eq!(delivered, 1, "Only the live token should count as delivered");
|
assert_eq!(
|
||||||
|
delivered, 1,
|
||||||
|
"Only the live token should count as delivered"
|
||||||
|
);
|
||||||
|
|
||||||
let all = db::push_tokens::list().unwrap();
|
let all = db::push_tokens::list().unwrap();
|
||||||
assert!(
|
assert!(
|
||||||
|
|||||||
@@ -335,4 +335,37 @@ mod tests {
|
|||||||
let openapi = <ApiDoc as OpenApi>::openapi();
|
let openapi = <ApiDoc as OpenApi>::openapi();
|
||||||
assert_eq!(openapi.info.version, env!("CARGO_PKG_VERSION"));
|
assert_eq!(openapi.info.version, env!("CARGO_PKG_VERSION"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn openapi_operation_ids_are_unique() {
|
||||||
|
// Every operation defaults its operationId to its handler function name, so handlers that
|
||||||
|
// share a name (e.g. each scanner's `status`, or `read`/`list` across resources) collide.
|
||||||
|
// Duplicate operationIds are invalid OpenAPI and make Swagger UI's "Try it out" execute the
|
||||||
|
// first operation with that id (so the DHCP doc hit `/api/arp_scanner/status`). Assert they
|
||||||
|
// are all unique so that regression cannot return.
|
||||||
|
let openapi = <ApiDoc as OpenApi>::openapi();
|
||||||
|
let mut seen = std::collections::HashSet::new();
|
||||||
|
for (path, item) in &openapi.paths.paths {
|
||||||
|
let operations = [
|
||||||
|
&item.get,
|
||||||
|
&item.put,
|
||||||
|
&item.post,
|
||||||
|
&item.delete,
|
||||||
|
&item.options,
|
||||||
|
&item.head,
|
||||||
|
&item.patch,
|
||||||
|
&item.trace,
|
||||||
|
];
|
||||||
|
for operation in operations.into_iter().flatten() {
|
||||||
|
let id = operation
|
||||||
|
.operation_id
|
||||||
|
.as_ref()
|
||||||
|
.unwrap_or_else(|| panic!("{path} has an operation with no operationId"));
|
||||||
|
assert!(
|
||||||
|
seen.insert(id.clone()),
|
||||||
|
"duplicate operationId {id:?} (at {path})"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ use crate::web_server::scanner_status::{ActiveScannerStatusResponse, active_resp
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/arp_scanner/status",
|
path = "/api/arp_scanner/status",
|
||||||
|
operation_id = "arp_scanner_status",
|
||||||
tag = "arp_scanner",
|
tag = "arp_scanner",
|
||||||
responses(
|
responses(
|
||||||
(status = 200, description = "ARP scanner status", body = ActiveScannerStatusResponse),
|
(status = 200, description = "ARP scanner status", body = ActiveScannerStatusResponse),
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ use crate::settings::get_settings;
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/config",
|
path = "/api/config",
|
||||||
|
operation_id = "config_read",
|
||||||
tag = "config",
|
tag = "config",
|
||||||
responses(
|
responses(
|
||||||
(status = 200, description = "Front-end configuration", body = Config),
|
(status = 200, description = "Front-end configuration", body = Config),
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ use crate::{db, model::device_events::DeviceEvent, web_server::utils};
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/devices/{mac_address}/events",
|
path = "/api/devices/{mac_address}/events",
|
||||||
|
operation_id = "device_events_list",
|
||||||
tag = "device_events",
|
tag = "device_events",
|
||||||
params(
|
params(
|
||||||
("mac_address" = String, Path, description = "MAC address of the device"),
|
("mac_address" = String, Path, description = "MAC address of the device"),
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ use crate::web_server::utils;
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/devices",
|
path = "/api/devices",
|
||||||
|
operation_id = "devices_list",
|
||||||
tag = "devices",
|
tag = "devices",
|
||||||
params(
|
params(
|
||||||
("is_registered" = Option<bool>, Query, description = "Filter by registration status"),
|
("is_registered" = Option<bool>, Query, description = "Filter by registration status"),
|
||||||
@@ -95,6 +96,7 @@ pub async fn list(
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/devices/{mac_address}",
|
path = "/api/devices/{mac_address}",
|
||||||
|
operation_id = "devices_read",
|
||||||
tag = "devices",
|
tag = "devices",
|
||||||
params(
|
params(
|
||||||
("mac_address" = String, Path, description = "MAC address of the device"),
|
("mac_address" = String, Path, description = "MAC address of the device"),
|
||||||
@@ -116,6 +118,7 @@ pub async fn read(Path(mac_address): Path<String>) -> Result<Json<Device>, Statu
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
put,
|
put,
|
||||||
path = "/api/devices",
|
path = "/api/devices",
|
||||||
|
operation_id = "devices_register",
|
||||||
tag = "devices",
|
tag = "devices",
|
||||||
request_body = RegisterDevicePayload,
|
request_body = RegisterDevicePayload,
|
||||||
responses(
|
responses(
|
||||||
@@ -235,6 +238,7 @@ pub async fn update(
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
delete,
|
delete,
|
||||||
path = "/api/devices/{mac_address}",
|
path = "/api/devices/{mac_address}",
|
||||||
|
operation_id = "devices_unregister",
|
||||||
tag = "devices",
|
tag = "devices",
|
||||||
params(
|
params(
|
||||||
("mac_address" = String, Path, description = "MAC address of the device"),
|
("mac_address" = String, Path, description = "MAC address of the device"),
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ use crate::web_server::scanner_status::{PassiveScannerStatusResponse, passive_re
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/dhcp_scanner/status",
|
path = "/api/dhcp_scanner/status",
|
||||||
|
operation_id = "dhcp_scanner_status",
|
||||||
tag = "dhcp_scanner",
|
tag = "dhcp_scanner",
|
||||||
responses(
|
responses(
|
||||||
(status = 200, description = "DHCP scanner status", body = PassiveScannerStatusResponse),
|
(status = 200, description = "DHCP scanner status", body = PassiveScannerStatusResponse),
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ use crate::web_server::scanner_status::{PassiveScannerStatusResponse, passive_re
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/mdns_scanner/status",
|
path = "/api/mdns_scanner/status",
|
||||||
|
operation_id = "mdns_scanner_status",
|
||||||
tag = "mdns_scanner",
|
tag = "mdns_scanner",
|
||||||
responses(
|
responses(
|
||||||
(status = 200, description = "mDNS scanner status", body = PassiveScannerStatusResponse),
|
(status = 200, description = "mDNS scanner status", body = PassiveScannerStatusResponse),
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ pub struct TestNotificationResponse {
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/notifications/{id}",
|
path = "/api/notifications/{id}",
|
||||||
|
operation_id = "notifications_read",
|
||||||
tag = "notifications",
|
tag = "notifications",
|
||||||
params(
|
params(
|
||||||
("id" = i64, Path, description = "Notification ID"),
|
("id" = i64, Path, description = "Notification ID"),
|
||||||
@@ -150,6 +151,7 @@ pub async fn send_test() -> Result<Json<TestNotificationResponse>, StatusCode> {
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/notifications",
|
path = "/api/notifications",
|
||||||
|
operation_id = "notifications_list",
|
||||||
tag = "notifications",
|
tag = "notifications",
|
||||||
params(
|
params(
|
||||||
("is_new" = Option<bool>, Query, description = "Filter by new/read status"),
|
("is_new" = Option<bool>, Query, description = "Filter by new/read status"),
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ use crate::model::push_tokens::PushPlatform;
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
put,
|
put,
|
||||||
path = "/api/push_tokens",
|
path = "/api/push_tokens",
|
||||||
|
operation_id = "push_tokens_register",
|
||||||
tag = "push_tokens",
|
tag = "push_tokens",
|
||||||
request_body = RegisterPushTokenPayload,
|
request_body = RegisterPushTokenPayload,
|
||||||
responses(
|
responses(
|
||||||
@@ -38,6 +39,7 @@ pub async fn register(Json(payload): Json<RegisterPushTokenPayload>) -> impl Int
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
delete,
|
delete,
|
||||||
path = "/api/push_tokens/{token}",
|
path = "/api/push_tokens/{token}",
|
||||||
|
operation_id = "push_tokens_unregister",
|
||||||
tag = "push_tokens",
|
tag = "push_tokens",
|
||||||
params(
|
params(
|
||||||
("token" = String, Path, description = "The push token to unregister"),
|
("token" = String, Path, description = "The push token to unregister"),
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ use crate::web_server::scanner_status::{ActiveScannerStatusResponse, active_resp
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/snmp_scanner/status",
|
path = "/api/snmp_scanner/status",
|
||||||
|
operation_id = "snmp_scanner_status",
|
||||||
tag = "snmp_scanner",
|
tag = "snmp_scanner",
|
||||||
responses(
|
responses(
|
||||||
(status = 200, description = "SNMP scanner status", body = ActiveScannerStatusResponse),
|
(status = 200, description = "SNMP scanner status", body = ActiveScannerStatusResponse),
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ use crate::web_server::scanner_status::{PassiveScannerStatusResponse, passive_re
|
|||||||
#[utoipa::path(
|
#[utoipa::path(
|
||||||
get,
|
get,
|
||||||
path = "/api/ssdp_scanner/status",
|
path = "/api/ssdp_scanner/status",
|
||||||
|
operation_id = "ssdp_scanner_status",
|
||||||
tag = "ssdp_scanner",
|
tag = "ssdp_scanner",
|
||||||
responses(
|
responses(
|
||||||
(status = 200, description = "SSDP scanner status", body = PassiveScannerStatusResponse),
|
(status = 200, description = "SSDP scanner status", body = PassiveScannerStatusResponse),
|
||||||
|
|||||||
Reference in New Issue
Block a user