aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-04-04 16:32:16 -0500
committerCraig Jennings <c@cjennings.net>2026-04-04 16:32:16 -0500
commit73c81a00a10766900318d86640249d1b54c6b351 (patch)
tree793f9c858060591c34813af05e84c7a6a5442153
parenta77a7b86f45ae96ff1802ea6f8b87dafd46b17b0 (diff)
downloademacs-wttrin-73c81a00a10766900318d86640249d1b54c6b351.tar.gz
emacs-wttrin-73c81a00a10766900318d86640249d1b54c6b351.zip
feat: specific error messages for fetch failures
Add HTTP status code checking (wttrin--extract-http-status) and pass error descriptions through the callback chain so users see "Location not found (HTTP 404)" or "Network error — check your connection" instead of the generic "Perhaps the location was misspelled?" for every failure. Also fix pre-existing bug where the condition-case error handler in extract-response-body killed an unrelated buffer after unwind-protect already cleaned up. 330 tests (was 307), all passing.
-rw-r--r--.stignore1
-rw-r--r--tests/test-wttrin--extract-http-status.el61
-rw-r--r--tests/test-wttrin--extract-response-body.el28
-rw-r--r--tests/test-wttrin--fetch-url.el16
-rw-r--r--tests/test-wttrin--get-cached-or-fetch.el20
-rw-r--r--tests/test-wttrin--handle-fetch-callback.el72
-rw-r--r--tests/test-wttrin-ansi-color-rendering.el2
-rw-r--r--tests/test-wttrin-error-propagation.el231
-rw-r--r--tests/test-wttrin-fetch-raw-string.el2
-rw-r--r--wttrin-debug.el2
-rw-r--r--wttrin.el94
11 files changed, 466 insertions, 63 deletions
diff --git a/.stignore b/.stignore
new file mode 100644
index 0000000..6b8710a
--- /dev/null
+++ b/.stignore
@@ -0,0 +1 @@
+.git
diff --git a/tests/test-wttrin--extract-http-status.el b/tests/test-wttrin--extract-http-status.el
new file mode 100644
index 0000000..4a714e2
--- /dev/null
+++ b/tests/test-wttrin--extract-http-status.el
@@ -0,0 +1,61 @@
+;;; test-wttrin--extract-http-status.el --- Tests for wttrin--extract-http-status -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2025-2026 Craig Jennings
+
+;;; Commentary:
+
+;; Unit tests for wttrin--extract-http-status function.
+;; Parses the HTTP status code from url-retrieve's response buffer.
+
+;;; Code:
+
+(require 'ert)
+(require 'wttrin)
+(require 'testutil-wttrin)
+
+;;; Normal Cases
+
+(ert-deftest test-wttrin--extract-http-status-normal-200 ()
+ "Standard 200 OK response should return 200."
+ (with-temp-buffer
+ (insert "HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\n\r\nbody")
+ (should (= (wttrin--extract-http-status) 200))))
+
+(ert-deftest test-wttrin--extract-http-status-normal-404 ()
+ "404 Not Found response should return 404."
+ (with-temp-buffer
+ (insert "HTTP/1.1 404 Not Found\r\n\r\nNot found")
+ (should (= (wttrin--extract-http-status) 404))))
+
+(ert-deftest test-wttrin--extract-http-status-normal-500 ()
+ "500 Internal Server Error should return 500."
+ (with-temp-buffer
+ (insert "HTTP/1.1 500 Internal Server Error\r\n\r\nerror")
+ (should (= (wttrin--extract-http-status) 500))))
+
+;;; Boundary Cases
+
+(ert-deftest test-wttrin--extract-http-status-boundary-no-status-line ()
+ "Buffer with no HTTP status line should return nil."
+ (with-temp-buffer
+ (insert "just some text with no HTTP headers")
+ (should-not (wttrin--extract-http-status))))
+
+(ert-deftest test-wttrin--extract-http-status-boundary-http2 ()
+ "HTTP/2 responses use a different format but still have a status code."
+ (with-temp-buffer
+ (insert "HTTP/2 301 Moved Permanently\r\n\r\n")
+ (should (= (wttrin--extract-http-status) 301))))
+
+(ert-deftest test-wttrin--extract-http-status-boundary-does-not-move-point ()
+ "Parsing the status should not change point, so it doesn't interfere
+with subsequent header/body parsing."
+ (with-temp-buffer
+ (insert "HTTP/1.1 200 OK\r\n\r\nbody")
+ (goto-char (point-min))
+ (let ((pos-before (point)))
+ (wttrin--extract-http-status)
+ (should (= (point) pos-before)))))
+
+(provide 'test-wttrin--extract-http-status)
+;;; test-wttrin--extract-http-status.el ends here
diff --git a/tests/test-wttrin--extract-response-body.el b/tests/test-wttrin--extract-response-body.el
index 8dcf2d2..8e6686c 100644
--- a/tests/test-wttrin--extract-response-body.el
+++ b/tests/test-wttrin--extract-response-body.el
@@ -163,5 +163,33 @@
(should-not (buffer-live-p test-buffer))))
(should (string= "data" result))))
+;;; HTTP Status Code Handling
+;; Note: wttrin--extract-response-body kills its buffer, so we capture
+;; the result via a let binding before the buffer disappears.
+
+(ert-deftest test-wttrin--extract-response-body-error-404-returns-nil ()
+ "A 404 response should return nil — the location doesn't exist."
+ (let (result)
+ (with-current-buffer (generate-new-buffer " *test-404*")
+ (insert "HTTP/1.1 404 Not Found\r\nContent-Type: text/plain\r\n\r\nPage not found")
+ (setq result (wttrin--extract-response-body)))
+ (should-not result)))
+
+(ert-deftest test-wttrin--extract-response-body-error-500-returns-nil ()
+ "A 500 response should return nil — the server is broken."
+ (let (result)
+ (with-current-buffer (generate-new-buffer " *test-500*")
+ (insert "HTTP/1.1 500 Internal Server Error\r\n\r\nServer error")
+ (setq result (wttrin--extract-response-body)))
+ (should-not result)))
+
+(ert-deftest test-wttrin--extract-response-body-normal-200-still-returns-body ()
+ "A 200 response should still extract and return the body as before."
+ (let (result)
+ (with-current-buffer (generate-new-buffer " *test-200*")
+ (insert "HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\n\r\nWeather data here")
+ (setq result (wttrin--extract-response-body)))
+ (should (equal result "Weather data here"))))
+
(provide 'test-wttrin--extract-response-body)
;;; test-wttrin--extract-response-body.el ends here
diff --git a/tests/test-wttrin--fetch-url.el b/tests/test-wttrin--fetch-url.el
index 9a2e58b..4bb02d7 100644
--- a/tests/test-wttrin--fetch-url.el
+++ b/tests/test-wttrin--fetch-url.el
@@ -41,7 +41,7 @@
(wttrin--fetch-url
"http://example.com/weather"
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -61,7 +61,7 @@
(wttrin--fetch-url
"http://example.com/weather"
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-data data)))
(should (string-match-p "☀️" callback-data))
@@ -82,7 +82,7 @@
(wttrin--fetch-url
"http://example.com/weather"
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-data data)))
;; Headers should not be in the data
@@ -104,7 +104,7 @@
(wttrin--fetch-url
"http://example.com/weather"
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-data data)))
(should (string= "" callback-data)))))
@@ -122,7 +122,7 @@
(wttrin--fetch-url
"http://example.com/weather"
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-data data)))
(should (= 10000 (length callback-data))))))
@@ -141,7 +141,7 @@
(wttrin--fetch-url
"http://example.com/weather"
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-data data)))
(should (string= "Weather data" callback-data)))))
@@ -160,7 +160,7 @@
(wttrin--fetch-url
"http://example.com/weather"
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -180,7 +180,7 @@
(wttrin--fetch-url
"http://example.com/weather"
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)))
(should callback-called)
diff --git a/tests/test-wttrin--get-cached-or-fetch.el b/tests/test-wttrin--get-cached-or-fetch.el
index 75b4e8b..9bc393d 100644
--- a/tests/test-wttrin--get-cached-or-fetch.el
+++ b/tests/test-wttrin--get-cached-or-fetch.el
@@ -59,7 +59,7 @@
(wttrin--get-cached-or-fetch
location
- (lambda (data) (setq callback-result data)))
+ (lambda (data &optional _error-msg) (setq callback-result data)))
;; Should return cached data immediately
(should (equal callback-result test-wttrin--get-cached-or-fetch-sample-weather))
@@ -88,7 +88,7 @@
(wttrin--get-cached-or-fetch
location
- (lambda (data) (setq callback-result data)))
+ (lambda (data &optional _error-msg) (setq callback-result data)))
;; Should call fetch
(should fetch-called)
@@ -125,7 +125,7 @@ Proactive refresh keeps data fresh; on-demand reads always use cache."
(wttrin--get-cached-or-fetch
location
- (lambda (data) (setq callback-result data)))
+ (lambda (data &optional _error-msg) (setq callback-result data)))
;; Should serve old data without fetching
(should-not fetch-called)
@@ -160,7 +160,7 @@ Proactive refresh keeps data fresh; on-demand reads always use cache."
(wttrin--get-cached-or-fetch
location
- (lambda (data) (setq callback-result data)))
+ (lambda (data &optional _error-msg) (setq callback-result data)))
;; Should fetch despite fresh cache
(should fetch-called)
@@ -189,7 +189,7 @@ Proactive refresh keeps data fresh; on-demand reads always use cache."
(wttrin--get-cached-or-fetch
location
- (lambda (data) (setq callback-result data)))
+ (lambda (data &optional _error-msg) (setq callback-result data)))
;; Should fetch
(should fetch-called)
@@ -225,14 +225,14 @@ Proactive refresh keeps data fresh; on-demand reads always use cache."
(let ((wttrin-unit-system "m"))
(wttrin--get-cached-or-fetch
location
- (lambda (data) (setq callback-result data)))
+ (lambda (data &optional _error-msg) (setq callback-result data)))
(should (equal callback-result metric-data)))
;; Fetch with imperial - should get imperial cache
(let ((wttrin-unit-system "u"))
(wttrin--get-cached-or-fetch
location
- (lambda (data) (setq callback-result data)))
+ (lambda (data &optional _error-msg) (setq callback-result data)))
(should (equal callback-result imperial-data)))))
(test-wttrin--get-cached-or-fetch-teardown)))
@@ -266,7 +266,7 @@ Proactive refresh keeps data fresh; on-demand reads always use cache."
(wttrin--get-cached-or-fetch
location
- (lambda (data) (setq callback-result data)))
+ (lambda (data &optional _error-msg) (setq callback-result data)))
;; Should fall back to stale cache
(should (equal callback-result test-wttrin--get-cached-or-fetch-sample-weather))
@@ -296,7 +296,7 @@ Proactive refresh keeps data fresh; on-demand reads always use cache."
(wttrin--get-cached-or-fetch
location
- (lambda (data) (setq callback-result data)))
+ (lambda (data &optional _error-msg) (setq callback-result data)))
;; Should return nil (no fallback available)
(should (null callback-result))))
@@ -321,7 +321,7 @@ Proactive refresh keeps data fresh; on-demand reads always use cache."
(wttrin--get-cached-or-fetch
location
- (lambda (data) (setq callback-result data)))
+ (lambda (data &optional _error-msg) (setq callback-result data)))
;; Should attempt to fetch (nil is a valid location input)
(should fetch-called)
diff --git a/tests/test-wttrin--handle-fetch-callback.el b/tests/test-wttrin--handle-fetch-callback.el
index fb78736..d4158ac 100644
--- a/tests/test-wttrin--handle-fetch-callback.el
+++ b/tests/test-wttrin--handle-fetch-callback.el
@@ -33,7 +33,7 @@
(lambda () "Weather: ☀️ Sunny")))
(wttrin--handle-fetch-callback
nil ;; status with no error
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -48,7 +48,7 @@
(lambda () "")))
(wttrin--handle-fetch-callback
nil
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -64,7 +64,7 @@
(lambda () large-data)))
(wttrin--handle-fetch-callback
nil
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -81,7 +81,7 @@
(lambda () "data")))
(wttrin--handle-fetch-callback
nil ;; nil status means success
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)))
(should callback-called))))
@@ -94,7 +94,7 @@
(lambda () "data")))
(wttrin--handle-fetch-callback
'() ;; empty plist, no error key
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -108,7 +108,7 @@
(lambda () "data")))
(wttrin--handle-fetch-callback
'(:peer "example.com" :redirect nil) ;; status with other keys
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)))
(should callback-called))))
@@ -121,7 +121,7 @@
(callback-data 'not-nil))
(wttrin--handle-fetch-callback
'(:error (error "Network unreachable"))
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -134,7 +134,7 @@
(callback-data 'not-nil))
(wttrin--handle-fetch-callback
'(:error (error "HTTP 404"))
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -147,7 +147,7 @@
(callback-data 'not-nil))
(wttrin--handle-fetch-callback
'(:error (error "Request timed out"))
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -164,7 +164,7 @@
(condition-case err
(wttrin--handle-fetch-callback
nil
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(error "User callback error")))
(error
@@ -183,7 +183,7 @@
(condition-case err
(wttrin--handle-fetch-callback
nil
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)))
(error
(setq error-caught t)))
@@ -200,7 +200,7 @@
(lambda () nil)))
(wttrin--handle-fetch-callback
nil
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -213,7 +213,7 @@
(callback-data 'not-nil))
(wttrin--handle-fetch-callback
'(:error (error "First error") :another-error "Second error")
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-called t)
(setq callback-data data)))
@@ -221,5 +221,51 @@
;; Should return nil when :error key is present
(should (null callback-data))))
+;;; User-facing error messages
+
+(ert-deftest test-wttrin--handle-fetch-callback-error-network-shows-message ()
+ "Network errors should show a specific message in the echo area,
+not leave the user guessing."
+ (let ((displayed-message nil))
+ (cl-letf (((symbol-function 'wttrin--extract-response-body)
+ (lambda () nil))
+ ((symbol-function 'message)
+ (lambda (fmt &rest args)
+ (setq displayed-message (apply #'format fmt args)))))
+ (wttrin--handle-fetch-callback
+ '(:error (error "Network unreachable"))
+ #'ignore)
+ (should displayed-message)
+ (should (string-match-p "network" (downcase displayed-message))))))
+
+(ert-deftest test-wttrin--handle-fetch-callback-error-http-404-shows-message ()
+ "HTTP 404 should tell the user the location wasn't found."
+ (let ((displayed-message nil))
+ (cl-letf (((symbol-function 'wttrin--extract-response-body)
+ (lambda () nil))
+ ((symbol-function 'wttrin--extract-http-status)
+ (lambda () 404))
+ ((symbol-function 'message)
+ (lambda (fmt &rest args)
+ (setq displayed-message (apply #'format fmt args)))))
+ ;; No :error in status — url-retrieve succeeded but server returned 404
+ (wttrin--handle-fetch-callback nil #'ignore)
+ (should displayed-message)
+ (should (string-match-p "not found\\|404" (downcase displayed-message))))))
+
+(ert-deftest test-wttrin--handle-fetch-callback-error-http-500-shows-message ()
+ "HTTP 500 should tell the user the weather service had an error."
+ (let ((displayed-message nil))
+ (cl-letf (((symbol-function 'wttrin--extract-response-body)
+ (lambda () nil))
+ ((symbol-function 'wttrin--extract-http-status)
+ (lambda () 500))
+ ((symbol-function 'message)
+ (lambda (fmt &rest args)
+ (setq displayed-message (apply #'format fmt args)))))
+ (wttrin--handle-fetch-callback nil #'ignore)
+ (should displayed-message)
+ (should (string-match-p "service\\|server\\|500" (downcase displayed-message))))))
+
(provide 'test-wttrin--handle-fetch-callback)
;;; test-wttrin--handle-fetch-callback.el ends here
diff --git a/tests/test-wttrin-ansi-color-rendering.el b/tests/test-wttrin-ansi-color-rendering.el
index e2bf82b..accada9 100644
--- a/tests/test-wttrin-ansi-color-rendering.el
+++ b/tests/test-wttrin-ansi-color-rendering.el
@@ -316,7 +316,7 @@ This verifies the fetch function returns unfiltered data from wttr.in."
(testutil-wttrin-mock-http-response testutil-wttrin-sample-ansi-response
(wttrin-fetch-raw-string
location
- (lambda (data)
+ (lambda (data &optional _error-msg)
(setq callback-data data)))
;; Verify callback received ANSI codes
diff --git a/tests/test-wttrin-error-propagation.el b/tests/test-wttrin-error-propagation.el
new file mode 100644
index 0000000..56421fd
--- /dev/null
+++ b/tests/test-wttrin-error-propagation.el
@@ -0,0 +1,231 @@
+;;; test-wttrin-error-propagation.el --- Tests for error info through callbacks -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2025-2026 Craig Jennings
+
+;;; Commentary:
+
+;; Tests that error information flows from the fetch layer through callbacks
+;; to the display layer, so users see specific error messages instead of
+;; "Perhaps the location was misspelled?" for every failure.
+
+;;; Code:
+
+(require 'ert)
+(require 'wttrin)
+(require 'testutil-wttrin)
+
+;;; Setup and Teardown
+
+(defun test-wttrin-error-propagation-setup ()
+ "Setup for error propagation tests."
+ (testutil-wttrin-setup)
+ (when (get-buffer "*wttr.in*")
+ (kill-buffer "*wttr.in*")))
+
+(defun test-wttrin-error-propagation-teardown ()
+ "Teardown for error propagation tests."
+ (testutil-wttrin-teardown)
+ (when (get-buffer "*wttr.in*")
+ (kill-buffer "*wttr.in*")))
+
+;;; --------------------------------------------------------------------------
+;;; handle-fetch-callback passes error info to callback
+;;; --------------------------------------------------------------------------
+
+(ert-deftest test-wttrin-error-propagation-network-error-reaches-callback ()
+ "Network error description should be passed as second callback argument."
+ (let ((received-error nil))
+ (cl-letf (((symbol-function 'wttrin--extract-response-body)
+ (lambda () nil))
+ ((symbol-function 'message) #'ignore))
+ (wttrin--handle-fetch-callback
+ '(:error (error "Connection refused"))
+ (lambda (_data &optional error-msg)
+ (setq received-error error-msg)))
+ (should received-error)
+ (should (string-match-p "network" (downcase received-error))))))
+
+(ert-deftest test-wttrin-error-propagation-http-error-reaches-callback ()
+ "HTTP error description should be passed as second callback argument."
+ (let ((received-error nil))
+ (cl-letf (((symbol-function 'wttrin--extract-response-body)
+ (lambda () nil))
+ ((symbol-function 'wttrin--extract-http-status)
+ (lambda () 404))
+ ((symbol-function 'message) #'ignore))
+ (wttrin--handle-fetch-callback
+ nil
+ (lambda (_data &optional error-msg)
+ (setq received-error error-msg)))
+ (should received-error)
+ (should (string-match-p "not found\\|404" (downcase received-error))))))
+
+(ert-deftest test-wttrin-error-propagation-success-no-error ()
+ "Successful fetch should pass nil as the error argument."
+ (let ((received-error 'not-called))
+ (cl-letf (((symbol-function 'wttrin--extract-response-body)
+ (lambda () "weather data"))
+ ((symbol-function 'wttrin--extract-http-status)
+ (lambda () 200)))
+ (wttrin--handle-fetch-callback
+ nil
+ (lambda (_data &optional error-msg)
+ (setq received-error error-msg)))
+ (should-not received-error))))
+
+;;; --------------------------------------------------------------------------
+;;; get-cached-or-fetch forwards error to caller's callback
+;;; --------------------------------------------------------------------------
+
+(ert-deftest test-wttrin-error-propagation-cache-miss-forwards-error ()
+ "When fetch fails with no cache, the error message should reach the caller."
+ (test-wttrin-error-propagation-setup)
+ (unwind-protect
+ (let ((received-error nil))
+ (cl-letf (((symbol-function 'wttrin-fetch-raw-string)
+ (lambda (_query callback)
+ (funcall callback nil "Network error — check your connection"))))
+ (let ((wttrin--force-refresh t))
+ (wttrin--get-cached-or-fetch
+ "Paris"
+ (lambda (_data &optional error-msg)
+ (setq received-error error-msg)))
+ (should received-error)
+ (should (string-match-p "network" (downcase received-error))))))
+ (test-wttrin-error-propagation-teardown)))
+
+(ert-deftest test-wttrin-error-propagation-stale-cache-no-error ()
+ "When fetch fails but stale cache exists, serve the data without an error."
+ (test-wttrin-error-propagation-setup)
+ (unwind-protect
+ (let ((received-data nil)
+ (received-error 'not-called))
+ (testutil-wttrin-add-to-cache "Paris" "old weather" 9999)
+ (cl-letf (((symbol-function 'wttrin-fetch-raw-string)
+ (lambda (_query callback)
+ (funcall callback nil "Network error")))
+ ((symbol-function 'message) #'ignore))
+ (let ((wttrin--force-refresh t))
+ (wttrin--get-cached-or-fetch
+ "Paris"
+ (lambda (data &optional error-msg)
+ (setq received-data data)
+ (setq received-error error-msg)))
+ ;; Should get stale data, not an error
+ (should received-data)
+ (should-not received-error))))
+ (test-wttrin-error-propagation-teardown)))
+
+;;; --------------------------------------------------------------------------
+;;; display-weather shows specific error message
+;;; --------------------------------------------------------------------------
+
+(ert-deftest test-wttrin-error-propagation-display-shows-specific-error ()
+ "When an error message is provided, display-weather should show it
+instead of the generic 'Perhaps the location was misspelled?' text."
+ (test-wttrin-error-propagation-setup)
+ (unwind-protect
+ (let ((displayed-message nil))
+ (cl-letf (((symbol-function 'message)
+ (lambda (fmt &rest args)
+ (setq displayed-message (apply #'format fmt args)))))
+ (wttrin--display-weather "Paris" nil "Network error — check your connection")
+ (should displayed-message)
+ (should (string-match-p "network" (downcase displayed-message)))
+ ;; Should NOT show the generic message
+ (should-not (string-match-p "misspelled" (downcase displayed-message)))))
+ (test-wttrin-error-propagation-teardown)))
+
+(ert-deftest test-wttrin-error-propagation-display-falls-back-to-generic ()
+ "When no error message is provided, display-weather should show the
+generic message for backward compatibility."
+ (test-wttrin-error-propagation-setup)
+ (unwind-protect
+ (let ((displayed-message nil))
+ (cl-letf (((symbol-function 'message)
+ (lambda (fmt &rest args)
+ (setq displayed-message (apply #'format fmt args)))))
+ (wttrin--display-weather "Paris" nil)
+ (should displayed-message)
+ (should (string-match-p "Cannot retrieve" displayed-message))))
+ (test-wttrin-error-propagation-teardown)))
+
+;;; --------------------------------------------------------------------------
+;;; End-to-end: full query chain
+;;; --------------------------------------------------------------------------
+
+(ert-deftest test-wttrin-error-propagation-e2e-404-shows-not-found ()
+ "Full chain: user queries a bad location, API returns 404, user sees
+'not found' — not the generic 'misspelled' message."
+ (test-wttrin-error-propagation-setup)
+ (unwind-protect
+ (let ((displayed-message nil))
+ (cl-letf (((symbol-function 'url-retrieve)
+ (lambda (_url callback)
+ (with-current-buffer (generate-new-buffer " *test-404*")
+ (insert "HTTP/1.1 404 Not Found\r\n\r\nUnknown location")
+ (funcall callback nil))))
+ ((symbol-function 'message)
+ (lambda (fmt &rest args)
+ (setq displayed-message (apply #'format fmt args)))))
+ (wttrin-query "Nonexistent Place")
+ (should displayed-message)
+ (should (string-match-p "not found\\|404" (downcase displayed-message)))))
+ (test-wttrin-error-propagation-teardown)))
+
+(ert-deftest test-wttrin-error-propagation-e2e-network-error-shows-network ()
+ "Full chain: network is down, user sees 'network error'."
+ (test-wttrin-error-propagation-setup)
+ (unwind-protect
+ (let ((displayed-message nil))
+ (cl-letf (((symbol-function 'url-retrieve)
+ (lambda (_url callback)
+ (with-temp-buffer
+ (funcall callback '(:error (error "Connection refused"))))))
+ ((symbol-function 'message)
+ (lambda (fmt &rest args)
+ (setq displayed-message (apply #'format fmt args)))))
+ (let ((wttrin--force-refresh t))
+ (wttrin-query "Paris"))
+ (should displayed-message)
+ (should (string-match-p "network" (downcase displayed-message)))))
+ (test-wttrin-error-propagation-teardown)))
+
+;;; --------------------------------------------------------------------------
+;;; Passthrough layers
+;;; --------------------------------------------------------------------------
+
+(ert-deftest test-wttrin-error-propagation-fetch-raw-string-forwards-error ()
+ "wttrin-fetch-raw-string is a passthrough — error-msg from the fetch
+layer must not be dropped."
+ (let ((received-error nil))
+ (cl-letf (((symbol-function 'wttrin--fetch-url)
+ (lambda (_url callback)
+ (funcall callback nil "test error message"))))
+ (wttrin-fetch-raw-string
+ "Paris"
+ (lambda (_data &optional error-msg)
+ (setq received-error error-msg)))
+ ;; fetch-raw-string calls fetch-url which calls handle-fetch-callback
+ ;; which calls our callback — but we mocked fetch-url to call directly,
+ ;; so the error-msg should pass through
+ (should (equal received-error "test error message")))))
+
+(ert-deftest test-wttrin-error-propagation-http-3xx-shows-unexpected ()
+ "HTTP 3xx that reaches us (unusual — url-retrieve follows redirects)
+should still produce an error message, not silently return nil."
+ (let ((received-error nil))
+ (cl-letf (((symbol-function 'wttrin--extract-response-body)
+ (lambda () nil))
+ ((symbol-function 'wttrin--extract-http-status)
+ (lambda () 301))
+ ((symbol-function 'message) #'ignore))
+ (wttrin--handle-fetch-callback
+ nil
+ (lambda (_data &optional error-msg)
+ (setq received-error error-msg)))
+ (should received-error)
+ (should (string-match-p "301\\|unexpected" (downcase received-error))))))
+
+(provide 'test-wttrin-error-propagation)
+;;; test-wttrin-error-propagation.el ends here
diff --git a/tests/test-wttrin-fetch-raw-string.el b/tests/test-wttrin-fetch-raw-string.el
index 34db7f9..e775025 100644
--- a/tests/test-wttrin-fetch-raw-string.el
+++ b/tests/test-wttrin-fetch-raw-string.el
@@ -47,7 +47,7 @@
(lambda (_url callback)
(funcall callback "weather response"))))
(wttrin-fetch-raw-string "Paris"
- (lambda (data) (setq received-data data)))
+ (lambda (data &optional _error-msg) (setq received-data data)))
(should (equal received-data "weather response"))))
(test-wttrin-fetch-raw-string-teardown)))
diff --git a/wttrin-debug.el b/wttrin-debug.el
index d0473dd..53f7e29 100644
--- a/wttrin-debug.el
+++ b/wttrin-debug.el
@@ -45,7 +45,7 @@ Always fetches fresh data from the API, bypassing cache."
(let ((wttrin--force-refresh t))
(wttrin--get-cached-or-fetch
location
- (lambda (raw-string)
+ (lambda (raw-string &optional _error-msg)
(with-current-buffer (get-buffer-create "*wttrin-debug*")
(erase-buffer)
(when raw-string
diff --git a/wttrin.el b/wttrin.el
index b3a5017..b628b1c 100644
--- a/wttrin.el
+++ b/wttrin.el
@@ -249,43 +249,75 @@ Returns \"just now\" for <60s, \"X minutes ago\", \"X hours ago\", or \"X days a
(wttrin-additional-url-params)
"A"))
+(defun wttrin--extract-http-status ()
+ "Return the HTTP status code from the current buffer, or nil.
+Reads the status line without moving point."
+ (save-excursion
+ (goto-char (point-min))
+ (when (re-search-forward "^HTTP/[0-9.]+ \\([0-9]+\\)" nil t)
+ (string-to-number (match-string 1)))))
+
(defun wttrin--extract-response-body ()
"Extract and decode HTTP response body from current buffer.
Skips headers and returns UTF-8 decoded body.
-Returns nil on error. Kills buffer when done."
+Returns nil for non-2xx status codes or on error. Kills buffer when done."
(condition-case err
(unwind-protect
- (progn
- (goto-char (point-min))
- ;; Skip past HTTP headers — blank line separates headers from body
- (re-search-forward "\r?\n\r?\n" nil t)
- (let ((body (decode-coding-string
- (buffer-substring-no-properties (point) (point-max))
- 'utf-8)))
- (wttrin--debug-log "wttrin--extract-response-body: Successfully fetched %d bytes"
- (length body))
- body))
+ (let ((status (wttrin--extract-http-status)))
+ (if (and status (>= status 300))
+ (progn
+ (wttrin--debug-log "wttrin--extract-response-body: HTTP %d" status)
+ nil)
+ (goto-char (point-min))
+ ;; Skip past HTTP headers — blank line separates headers from body
+ (re-search-forward "\r?\n\r?\n" nil t)
+ (let ((body (decode-coding-string
+ (buffer-substring-no-properties (point) (point-max))
+ 'utf-8)))
+ (wttrin--debug-log "wttrin--extract-response-body: Successfully fetched %d bytes"
+ (length body))
+ body)))
+ ;; unwind-protect handles buffer cleanup for all paths
(ignore-errors (kill-buffer (current-buffer))))
(error
(wttrin--debug-log "wttrin--extract-response-body: Error - %s"
(error-message-string err))
- (ignore-errors (kill-buffer (current-buffer)))
nil)))
(defun wttrin--handle-fetch-callback (status callback)
"Handle `url-retrieve' callback STATUS and invoke CALLBACK with result.
-Extracts response body or handles errors, then calls CALLBACK with data or nil."
+Calls CALLBACK with (DATA &optional ERROR-MSG). DATA is the response
+body string on success, nil on failure. ERROR-MSG is a human-readable
+description of what went wrong, or nil on success."
(wttrin--debug-log "wttrin--handle-fetch-callback: Invoked with status = %S" status)
- (let ((data nil))
- (if (plist-get status :error)
- (wttrin--debug-log "wttrin--handle-fetch-callback: Network error - %s"
- (cdr (plist-get status :error)))
- (setq data (wttrin--extract-response-body)))
+ (let ((data nil)
+ (error-msg nil))
+ (cond
+ ;; Network-level failure (DNS, connection refused, timeout)
+ ((plist-get status :error)
+ (wttrin--debug-log "wttrin--handle-fetch-callback: Network error - %s"
+ (cdr (plist-get status :error)))
+ (setq error-msg "Network error — check your connection")
+ (message "wttrin: %s" error-msg))
+ ;; HTTP response received — extract body (returns nil for non-2xx)
+ (t
+ (let ((http-status (wttrin--extract-http-status)))
+ (setq data (wttrin--extract-response-body))
+ (when (and (not data) http-status)
+ (setq error-msg
+ (cond
+ ((and (>= http-status 400) (< http-status 500))
+ (format "Location not found (HTTP %d)" http-status))
+ ((>= http-status 500)
+ (format "Weather service error (HTTP %d)" http-status))
+ (t (format "Unexpected HTTP status %d" http-status))))
+ (when error-msg
+ (message "wttrin: %s" error-msg))))))
(condition-case err
(progn
(wttrin--debug-log "wttrin--handle-fetch-callback: Calling user callback with %s"
(if data (format "%d bytes" (length data)) "nil"))
- (funcall callback data))
+ (funcall callback data error-msg))
(error
(wttrin--debug-log "wttrin--handle-fetch-callback: Error in user callback - %s"
(error-message-string err))
@@ -394,13 +426,17 @@ Looks up the cache timestamp for LOCATION and formats a line like
(age-str (wttrin--format-age age)))
(format "Last updated: %s (%s)" (string-trim time-str) age-str)))))
-(defun wttrin--display-weather (location-name raw-string)
- "Display weather data RAW-STRING for LOCATION-NAME in weather buffer."
+(defun wttrin--display-weather (location-name raw-string &optional error-msg)
+ "Display weather data RAW-STRING for LOCATION-NAME in weather buffer.
+When ERROR-MSG is provided and data is invalid, show that instead of
+the generic error message."
(when wttrin-debug
(wttrin--save-debug-data location-name raw-string))
(if (not (wttrin--validate-weather-data raw-string))
- (message "Cannot retrieve weather data. Perhaps the location was misspelled?")
+ (message "wttrin: %s"
+ (or error-msg
+ "Cannot retrieve weather data. Perhaps the location was misspelled?"))
(let ((buffer (get-buffer-create (format "*wttr.in*"))))
(switch-to-buffer buffer)
@@ -434,10 +470,10 @@ Looks up the cache timestamp for LOCATION and formats a line like
(setq buffer-read-only t)
(wttrin--get-cached-or-fetch
location-name
- (lambda (raw-string)
+ (lambda (raw-string &optional error-msg)
(when (buffer-live-p buffer)
(with-current-buffer buffer
- (wttrin--display-weather location-name raw-string)))))))
+ (wttrin--display-weather location-name raw-string error-msg)))))))
(defun wttrin--make-cache-key (location)
"Create cache key from LOCATION and current settings."
@@ -447,7 +483,7 @@ Looks up the cache timestamp for LOCATION and formats a line like
"Get cached weather for LOCATION or fetch if not cached.
If cache has data and not force-refreshing, serves it immediately
regardless of age. The background refresh timer keeps data fresh.
-CALLBACK is called with the weather data string when ready, or nil on error."
+CALLBACK is called with (DATA &optional ERROR-MSG)."
(let* ((cache-key (wttrin--make-cache-key location))
(cached (gethash cache-key wttrin--cache))
(data (cdr cached)))
@@ -456,7 +492,7 @@ CALLBACK is called with the weather data string when ready, or nil on error."
(funcall callback data)
(wttrin-fetch-raw-string
location
- (lambda (fresh-data)
+ (lambda (fresh-data &optional error-msg)
(if fresh-data
(progn
(wttrin--cleanup-cache-if-needed)
@@ -467,7 +503,7 @@ CALLBACK is called with the weather data string when ready, or nil on error."
(progn
(message "Failed to fetch new data, using cached version")
(funcall callback data))
- (funcall callback nil))))))))
+ (funcall callback nil error-msg))))))))
(defun wttrin--get-cache-entries-by-age ()
"Return list of (key . timestamp) pairs sorted oldest-first.
@@ -578,7 +614,7 @@ On failure with no cache, shows error placeholder."
(wttrin--debug-log "mode-line-fetch: URL = %s" url)
(wttrin--fetch-url
url
- (lambda (data)
+ (lambda (data &optional _error-msg)
(if data
(let ((trimmed-data (string-trim data)))
(wttrin--debug-log "mode-line-fetch: Received data = %S" trimmed-data)
@@ -676,7 +712,7 @@ opens the weather buffer."
(cache-key (wttrin--make-cache-key location)))
(wttrin-fetch-raw-string
location
- (lambda (fresh-data)
+ (lambda (fresh-data &optional _error-msg)
(when fresh-data
(wttrin--cleanup-cache-if-needed)
(puthash cache-key (cons (float-time) fresh-data) wttrin--cache)))))))