Fix missing env var error #5

Merged
qpismont merged 4 commits from 1.0.1 into main 2026-06-16 22:05:25 +02:00
4 changed files with 13 additions and 13 deletions
Showing only changes of commit 00d46ce968 - Show all commits
Generated
+1 -1
View File
@@ -786,7 +786,7 @@ checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a"
[[package]]
name = "herald"
version = "1.0.0"
version = "1.0.1"
dependencies = [
"anyhow",
"axum",
+1 -1
View File
@@ -1,6 +1,6 @@
[package]
name = "herald"
version = "1.0.0"
version = "1.0.1"
edition = "2024"
[dependencies]
1
+3 -3
View File
@@ -41,13 +41,13 @@ pub fn load_config() -> anyhow::Result<EnvConfig> {
}
pub fn try_get_env(key: &str) -> anyhow::Result<String> {
let env = std::env::var(key).map_err(|e| anyhow::anyhow!(format!("{}: {}", key, e)))?;
let env_value = std::env::var(key).map_err(|e| anyhow::anyhow!("{}: {}", key, e))?;
qpismont marked this conversation as resolved
Review

Cette transformation en anyhow::Error est claire et appropriée. Bonne pratique pour la propagation d'erreur.

Cette transformation en `anyhow::Error` est claire et appropriée. Bonne pratique pour la propagation d'erreur.
if env.trim().is_empty() {
if env_value.trim().is_empty() {
qpismont marked this conversation as resolved
Review

Ce bloc if ne fait rien si la valeur est vide. Il semble que vous souhaitiez traiter une variable d'environnement vide comme une erreur, mais le code continue et retourne Ok(env_value) à la ligne 50. Vous devriez ajouter une instruction de retour d'erreur (par exemple avec bail! ou return Err(...)) à l'intérieur du bloc if, ou bien utiliser une structure différente pour gérer le cas vide.

Ce bloc `if` ne fait rien si la valeur est vide. Il semble que vous souhaitiez traiter une variable d'environnement vide comme une erreur, mais le code continue et retourne `Ok(env_value)` à la ligne 50. Vous devriez ajouter une instruction de retour d'erreur (par exemple avec `bail!` ou `return Err(...)`) à l'intérieur du bloc `if`, ou bien utiliser une structure différente pour gérer le cas vide.
Review

Après avoir vérifié que la variable d'environnement est vide ou ne contient que des espaces, il n'y a pas de traitement visible (pas de return Err(...) ni de panic!). Si l'intention est de rejeter les valeurs vides, il faut ajouter une gestion d'erreur explicite, par exemple return Err(anyhow::anyhow!("{}: variable vide", key)). Sinon, la fonction continue avec une valeur vide et échouera peut-être plus tard de manière obscure.

Après avoir vérifié que la variable d'environnement est vide ou ne contient que des espaces, il n'y a pas de traitement visible (pas de `return Err(...)` ni de `panic!`). Si l'intention est de rejeter les valeurs vides, il faut ajouter une gestion d'erreur explicite, par exemple `return Err(anyhow::anyhow!("{}: variable vide", key))`. Sinon, la fonction continue avec une valeur vide et échouera peut-être plus tard de manière obscure.
return Err(anyhow!(format!("env var {} is empty", key)));
}
Ok(env)
Ok(env_value)
}
#[cfg(test)]
+8 -8
View File
@@ -206,14 +206,6 @@ impl WebhookType {
_ => Err(AppError::UnknownEventErr),
}?;
let pr_body = match &wb {
WebhookType::Review(review_payload) => &review_payload.comment.body,
};
if !pr_body.starts_with(&format!("@{}", bot_name)) {
return Err(AppError::UnauthorizedUserErr);
}
let action = match &wb {
WebhookType::Review(review_payload) => &review_payload.action,
};
@@ -222,6 +214,14 @@ impl WebhookType {
return Err(AppError::InvalidActionErr);
}
let pr_body = match &wb {
qpismont marked this conversation as resolved
Review

Le motif match ne gère que la variante WebhookType::Review. Si une autre variante (par exemple WebhookType::PullRequest) est reçue, cela provoquera une panique. Il faut soit étendre le match à toutes les variantes, soit s'assurer que cette fonction n'est appelée que pour des événements Review (par exemple en déplaçant la logique dans le bloc if let).

Le motif `match` ne gère que la variante `WebhookType::Review`. Si une autre variante (par exemple `WebhookType::PullRequest`) est reçue, cela provoquera une panique. Il faut soit étendre le match à toutes les variantes, soit s'assurer que cette fonction n'est appelée que pour des événements `Review` (par exemple en déplaçant la logique dans le bloc `if let`).
WebhookType::Review(review_payload) => &review_payload.comment.body,
qpismont marked this conversation as resolved
Review

L'extraction du corps du commentaire via &review_payload.comment.body suppose que comment et body existent toujours. Si comment est optionnel ou que body peut être None, cela pourrait causer une panique. Envisagez d'utiliser une approche plus sécurisée avec des if let ou pattern matching pour gérer les valeurs manquantes.

L'extraction du corps du commentaire via `&review_payload.comment.body` suppose que `comment` et `body` existent toujours. Si `comment` est optionnel ou que `body` peut être `None`, cela pourrait causer une panique. Envisagez d'utiliser une approche plus sécurisée avec des `if let` ou pattern matching pour gérer les valeurs manquantes.
};
if !pr_body.starts_with(&format!("@{}", bot_name)) {
qpismont marked this conversation as resolved
Review

La variable bot_name n'est pas définie dans le contexte visible. Assurez-vous qu'elle est passée en paramètre ou accessible dans la portée, sinon cela causera une erreur de compilation.

La variable `bot_name` n'est pas définie dans le contexte visible. Assurez-vous qu'elle est passée en paramètre ou accessible dans la portée, sinon cela causera une erreur de compilation.
return Err(AppError::UnauthorizedUserErr);
}
Ok(wb)
}
}