Don't send emails when pinging Uptime Kuma fails due to a Kuma bug

[cdzombak]

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of duplicate entry responses to suppress error notifications in these scenarios, allowing the system to gracefully manage these cases without interrupting user experience.

[coderabbitai]

Walkthrough

Modified deliverSuccessNotification in deliver.go to add conditional error suppression for 404 responses. When a 404 response body contains both "ok":false and "Duplicate entry", the function now returns success instead of propagating the error.

Changes

Cohort / File(s)	Summary
Error handling logic deliver.go	Added special-case handling for 404 responses that contain "ok":false and "Duplicate entry" strings in the response body; now returns nil instead of erroring for these specific conditions

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant deliverSuccessNotification
    participant Response

    Caller->>deliverSuccessNotification: Call with response
    deliverSuccessNotification->>Response: Check status code
    
    alt Status is 404
        Response-->>deliverSuccessNotification: 404 status
        deliverSuccessNotification->>Response: Read body
        Response-->>deliverSuccessNotification: Body content
        
        alt Contains "ok":false AND "Duplicate entry"
            rect rgb(144, 238, 144)
                deliverSuccessNotification-->>Caller: Return nil (success)
            end
        else
            rect rgb(255, 160, 160)
                deliverSuccessNotification-->>Caller: Return error
            end
        end
    else Status is not 404
        alt Is 2xx status
            rect rgb(144, 238, 144)
                deliverSuccessNotification-->>Caller: Return nil (success)
            end
        else Is non-2xx
            rect rgb(255, 160, 160)
                deliverSuccessNotification-->>Caller: Return error
            end
        end
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Focus on the string matching logic—verify that both "ok":false and "Duplicate entry" patterns are correctly detected in the response body
• Confirm the 404 status code check is the appropriate condition for this special case
• Consider whether this error suppression should apply to other status codes or response patterns

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title 'Don't send emails when pinging Uptime Kuma fails due to a Kuma bug' directly describes the main change in the code. The summary shows that the modification adds special-case handling to suppress errors for specific 404 responses from Uptime Kuma that contain 'Duplicate entry', which prevents error notifications (emails) from being sent when this particular Kuma bug occurs. The title clearly communicates this intent and is specific enough to convey the primary change.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch cdz/kuma-workaround

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

deliver.go (2)

265-265: Consider parsing the JSON response instead of string matching.

String matching on JSON content is fragile and could break if the response format changes (e.g., whitespace, field order). Parsing the JSON properly would be more robust.

Consider this approach with JSON parsing:

-		if strings.Contains(string(body), "ok\":false") && strings.Contains(string(body), "Duplicate entry") {
-			return nil
-		}
+		var jsonResp map[string]interface{}
+		if err := json.Unmarshal(body, &jsonResp); err == nil {
+			if ok, okExists := jsonResp["ok"].(bool); okExists && !ok {
+				if msg, msgExists := jsonResp["msg"].(string); msgExists && strings.Contains(msg, "Duplicate entry") {
+					return nil
+				}
+			}
+		}

Note: You may need to adjust the JSON field names based on the actual Uptime Kuma response structure.

---

261-268: Consider adding logging when the workaround is triggered.

When this workaround suppresses a 404 error, there's no indication in the logs. Adding a log message would improve observability and help track when the Uptime Kuma bug occurs, which would be useful for debugging and knowing when the workaround can be safely removed.

Example logging (adjust based on your logging framework):

if resp.StatusCode == 404 {
    body, err := io.ReadAll(resp.Body)
    if err != nil {
        return fmt.Errorf("failed to GET '%s' (%s, could not read response body: %w)", url, resp.Status, err)
    }
    if strings.Contains(string(body), "ok\":false") && strings.Contains(string(body), "Duplicate entry") {
        // TODO: Add appropriate logging here, e.g.:
        // log.Printf("Suppressing Uptime Kuma 404 error due to known bug (issue #5357) for URL: %s", url)
        return nil
    }
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 713fada and 413907e.

📒 Files selected for processing (1)

• deliver.go (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Binaries & Debian Packages

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Check the error from io.ReadAll.

Ignoring the error from io.ReadAll could mask I/O failures and lead to incorrect behavior. If reading the body fails, the workaround condition won't be evaluated correctly.

Apply this diff to handle the error:

-		body, _ := io.ReadAll(resp.Body)
+		body, err := io.ReadAll(resp.Body)
+		if err != nil {
+			return fmt.Errorf("failed to GET '%s' (%s, could not read response body: %w)", url, resp.Status, err)
+		}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	body, _ := io.ReadAll(resp.Body)
	body, err := io.ReadAll(resp.Body)
	if err != nil {
	return fmt.Errorf("failed to GET '%s' (%s, could not read response body: %w)", url, resp.Status, err)
	}

🤖 Prompt for AI Agents

In deliver.go around line 264, the io.ReadAll call ignores its error; change it
to capture the error (body, err := io.ReadAll(resp.Body)) and handle err: close
resp.Body if needed, log or wrap and return the error (or propagate it up) so
failures reading the response body don’t get silently ignored and the subsequent
workaround condition is evaluated against a valid body.