From 10772581000df33250c722abeb0a809e4a420631 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 06:20:57 -0500 Subject: [PATCH 01/10] feat: add connectivity pre-check on weather API failures When an API call to open-meteo.com fails (non-success status or exception), probe connectivitycheck.gstatic.com with a 2s timeout HEAD request to distinguish 'user is offline' from 'weather API unreachable/blocked'. - Add ProbeConnectivityAsync helper with 2s timeout - Probe on all 6 failure paths (3 status + 3 exception) - Add internal constructor for test injection - Rethrow OperationCanceledException instead of swallowing - Add 6 connectivity probe tests - Add resource strings for diagnostic messages Closes #58 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ConnectivityProbeTests.cs | 124 ++++++++++++++++++ .../Properties/Resources.Designer.cs | 45 +++++++ WeatherExtension/Properties/Resources.resx | 15 +++ WeatherExtension/Services/OpenMeteoService.cs | 54 ++++++++ 4 files changed, 238 insertions(+) create mode 100644 WeatherExtension.Tests/ConnectivityProbeTests.cs diff --git a/WeatherExtension.Tests/ConnectivityProbeTests.cs b/WeatherExtension.Tests/ConnectivityProbeTests.cs new file mode 100644 index 0000000..f2db8ae --- /dev/null +++ b/WeatherExtension.Tests/ConnectivityProbeTests.cs @@ -0,0 +1,124 @@ +// Copyright (c) Bald Bearded Builder LLC +// Bald Bearded Builder LLC licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Net; +using Microsoft.CmdPal.Ext.Weather.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.Ext.Weather.UnitTests; + +[TestClass] +public class ConnectivityProbeTests +{ + [TestMethod] + public async Task GetCurrentWeatherAsync_ApiReturns500_TriggersProbe() + { + var callCount = 0; + var handler = new CountingHttpHandler(request => + { + callCount++; + return new HttpResponseMessage(HttpStatusCode.InternalServerError); + }); + + using var service = new OpenMeteoService(handler); + var result = await service.GetCurrentWeatherAsync(52.52, 13.41, ct: CancellationToken.None); + + Assert.IsNull(result); + // 1 API call + 1 connectivity probe = 2 + Assert.AreEqual(2, callCount, "Expected API call + connectivity probe"); + } + + [TestMethod] + public async Task GetCurrentWeatherAsync_ApiThrows_TriggersProbe() + { + var callCount = 0; + var handler = new CountingHttpHandler(request => + { + callCount++; + throw new HttpRequestException("Connection refused"); + }); + + using var service = new OpenMeteoService(handler); + var result = await service.GetCurrentWeatherAsync(52.52, 13.41, ct: CancellationToken.None); + + Assert.IsNull(result); + // 1 failed API call + 1 probe attempt (also fails since same handler throws) = 2 + Assert.AreEqual(2, callCount, "Expected API call + connectivity probe attempt"); + } + + [TestMethod] + public async Task GetForecastAsync_ApiReturns500_TriggersProbe() + { + var callCount = 0; + var handler = new CountingHttpHandler(request => + { + callCount++; + return new HttpResponseMessage(HttpStatusCode.InternalServerError); + }); + + using var service = new OpenMeteoService(handler); + var result = await service.GetForecastAsync(52.52, 13.41, ct: CancellationToken.None); + + Assert.IsNull(result); + Assert.AreEqual(2, callCount, "Expected API call + connectivity probe"); + } + + [TestMethod] + public async Task GetHourlyForecastAsync_ApiReturns500_TriggersProbe() + { + var callCount = 0; + var handler = new CountingHttpHandler(request => + { + callCount++; + return new HttpResponseMessage(HttpStatusCode.InternalServerError); + }); + + using var service = new OpenMeteoService(handler); + var result = await service.GetHourlyForecastAsync(52.52, 13.41, ct: CancellationToken.None); + + Assert.IsNull(result); + Assert.AreEqual(2, callCount, "Expected API call + connectivity probe"); + } + + [TestMethod] + public async Task GetCurrentWeatherAsync_ApiSucceeds_NoProbe() + { + var callCount = 0; + var handler = new CountingHttpHandler(request => + { + callCount++; + return new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{}"), + }; + }); + + using var service = new OpenMeteoService(handler); + _ = await service.GetCurrentWeatherAsync(52.52, 13.41, ct: CancellationToken.None); + + // Only the API call, no probe + Assert.AreEqual(1, callCount, "Successful API call should not trigger probe"); + } + + [TestMethod] + public async Task GetCurrentWeatherAsync_Cancelled_NoProbe() + { + var callCount = 0; + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + var handler = new CountingHttpHandler(request => + { + callCount++; + throw new OperationCanceledException(); + }); + + using var service = new OpenMeteoService(handler); + await Assert.ThrowsExceptionAsync( + () => service.GetCurrentWeatherAsync(52.52, 13.41, ct: cts.Token)); + + // Cancelled — probe should not fire + Assert.AreEqual(1, callCount, "Cancelled request should not trigger probe"); + } +} diff --git a/WeatherExtension/Properties/Resources.Designer.cs b/WeatherExtension/Properties/Resources.Designer.cs index c278022..df58c53 100644 --- a/WeatherExtension/Properties/Resources.Designer.cs +++ b/WeatherExtension/Properties/Resources.Designer.cs @@ -61,6 +61,51 @@ internal Resources() { } /// + /// + /// Looks up a localized string similar to Weather API unreachable — may be blocked in your region. + /// + public static string connectivity_api_blocked { + get { + return ResourceManager.GetString("connectivity_api_blocked", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to current weather. + /// + public static string connectivity_endpoint_current_weather { + get { + return ResourceManager.GetString("connectivity_endpoint_current_weather", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to forecast. + /// + public static string connectivity_endpoint_forecast { + get { + return ResourceManager.GetString("connectivity_endpoint_forecast", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to hourly forecast. + /// + public static string connectivity_endpoint_hourly_forecast { + get { + return ResourceManager.GetString("connectivity_endpoint_hourly_forecast", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to No internet connection. + /// + public static string connectivity_no_internet { + get { + return ResourceManager.GetString("connectivity_no_internet", resourceCulture); + } + } + /// Looks up a localized string similar to Celsius. /// public static string celsius { diff --git a/WeatherExtension/Properties/Resources.resx b/WeatherExtension/Properties/Resources.resx index 4dd5f1c..55377d8 100644 --- a/WeatherExtension/Properties/Resources.resx +++ b/WeatherExtension/Properties/Resources.resx @@ -256,4 +256,19 @@ 12 hours + + No internet connection + + + Weather API unreachable — may be blocked in your region + + + current weather + + + forecast + + + hourly forecast + \ No newline at end of file diff --git a/WeatherExtension/Services/OpenMeteoService.cs b/WeatherExtension/Services/OpenMeteoService.cs index f86aacf..ca4f938 100644 --- a/WeatherExtension/Services/OpenMeteoService.cs +++ b/WeatherExtension/Services/OpenMeteoService.cs @@ -13,6 +13,7 @@ public sealed partial class OpenMeteoService : IWeatherService, IDisposable { private readonly HttpClient _httpClient; private const string BaseUrl = "https://api.open-meteo.com/v1/forecast"; + private const string ConnectivityProbeUrl = "https://connectivitycheck.gstatic.com/generate_204"; private const int CacheExpirationMinutes = 15; private WeatherData? _cachedWeather; @@ -36,6 +37,15 @@ public OpenMeteoService() _httpClient.DefaultRequestHeaders.Add("User-Agent", "PowerToys-CmdPal-Weather/1.0"); } + internal OpenMeteoService(HttpMessageHandler handler) + { + _httpClient = new HttpClient(handler) + { + Timeout = TimeSpan.FromSeconds(10), + }; + _httpClient.DefaultRequestHeaders.Add("User-Agent", "PowerToys-CmdPal-Weather/1.0"); + } + public async Task GetCurrentWeatherAsync( double latitude, double longitude, @@ -64,6 +74,7 @@ public OpenMeteoService() WeatherLogger.LogToHost( MessageState.Error, $"Weather API returned status {response.StatusCode}"); + await ProbeConnectivityAsync(Resources.connectivity_endpoint_current_weather, ct).ConfigureAwait(false); return null; } @@ -86,11 +97,16 @@ public OpenMeteoService() return weatherData; } + catch (OperationCanceledException) + { + throw; + } catch (Exception ex) { WeatherLogger.LogToHost( MessageState.Error, $"Weather fetch error: {ex.Message}"); + await ProbeConnectivityAsync(Resources.connectivity_endpoint_current_weather, ct).ConfigureAwait(false); return null; } } @@ -122,6 +138,7 @@ public OpenMeteoService() WeatherLogger.LogToHost( MessageState.Error, $"Forecast API returned status {response.StatusCode}"); + await ProbeConnectivityAsync(Resources.connectivity_endpoint_forecast, ct).ConfigureAwait(false); return null; } @@ -144,11 +161,16 @@ public OpenMeteoService() return forecastData; } + catch (OperationCanceledException) + { + throw; + } catch (Exception ex) { WeatherLogger.LogToHost( MessageState.Error, $"Forecast fetch error: {ex.Message}"); + await ProbeConnectivityAsync(Resources.connectivity_endpoint_forecast, ct).ConfigureAwait(false); return null; } } @@ -181,6 +203,7 @@ public OpenMeteoService() WeatherLogger.LogToHost( MessageState.Error, $"Hourly forecast API returned status {response.StatusCode}"); + await ProbeConnectivityAsync(Resources.connectivity_endpoint_hourly_forecast, ct).ConfigureAwait(false); return null; } @@ -203,15 +226,46 @@ public OpenMeteoService() return hourlyData; } + catch (OperationCanceledException) + { + throw; + } catch (Exception ex) { WeatherLogger.LogToHost( MessageState.Error, $"Hourly forecast fetch error: {ex.Message}"); + await ProbeConnectivityAsync(Resources.connectivity_endpoint_hourly_forecast, ct).ConfigureAwait(false); return null; } } + private async Task ProbeConnectivityAsync(string failedEndpoint, CancellationToken ct) + { + try + { + using var probeRequest = new HttpRequestMessage(HttpMethod.Head, ConnectivityProbeUrl); + probeRequest.Headers.Add("User-Agent", "PowerToys-CmdPal-Weather/1.0"); + + using var probeCts = CancellationTokenSource.CreateLinkedTokenSource(ct); + probeCts.CancelAfter(TimeSpan.FromSeconds(2)); + + var probeResponse = await _httpClient.SendAsync(probeRequest, probeCts.Token).ConfigureAwait(false); + + // Probe succeeded — we have internet, so the weather API is specifically unreachable + WeatherLogger.LogToHost( + MessageState.Error, + $"{Resources.connectivity_api_blocked} ({failedEndpoint})"); + } + catch + { + // Probe failed — no internet connection + WeatherLogger.LogToHost( + MessageState.Warning, + Resources.connectivity_no_internet); + } + } + public void Dispose() { _httpClient?.Dispose(); From eebdb4a6cd782c080567661ca025335ae84a273c Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 06:24:09 -0500 Subject: [PATCH 02/10] fix: correct expected call count in cancelled probe test Pre-cancelled token throws before handler lambda runs, so callCount is 0, not 1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- WeatherExtension.Tests/ConnectivityProbeTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WeatherExtension.Tests/ConnectivityProbeTests.cs b/WeatherExtension.Tests/ConnectivityProbeTests.cs index f2db8ae..549a1b3 100644 --- a/WeatherExtension.Tests/ConnectivityProbeTests.cs +++ b/WeatherExtension.Tests/ConnectivityProbeTests.cs @@ -119,6 +119,6 @@ await Assert.ThrowsExceptionAsync( () => service.GetCurrentWeatherAsync(52.52, 13.41, ct: cts.Token)); // Cancelled — probe should not fire - Assert.AreEqual(1, callCount, "Cancelled request should not trigger probe"); + Assert.AreEqual(0, callCount, "Cancelled request should not trigger probe"); } } From b114b42f0171234ed380dc7147ab8e1fb946cfb4 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 13:38:30 -0500 Subject: [PATCH 03/10] Fix CS0246: add missing using System.Threading.Tasks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- WeatherExtension.Tests/ConnectivityProbeTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/WeatherExtension.Tests/ConnectivityProbeTests.cs b/WeatherExtension.Tests/ConnectivityProbeTests.cs index 549a1b3..01f9dde 100644 --- a/WeatherExtension.Tests/ConnectivityProbeTests.cs +++ b/WeatherExtension.Tests/ConnectivityProbeTests.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Net; +using System.Threading.Tasks; using Microsoft.CmdPal.Ext.Weather.Services; using Microsoft.VisualStudio.TestTools.UnitTesting; From 7cffae6379ad483bacd368ec4cc710c746104236 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 13:38:56 -0500 Subject: [PATCH 04/10] fix: address Copilot review on ProbeConnectivityAsync and Resources.Designer.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix malformed XML doc comments in Resources.Designer.cs (duplicate before connectivity_api_blocked; missing before celsius) - Validate probe returns HTTP 204 (NoContent) before logging API-blocked — captive portals returning 200/302 now treated as no-internet - Split bare catch in ProbeConnectivityAsync: caller cancellation (ct fired) returns silently; probe timeout and network errors log no-internet Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Properties/Resources.Designer.cs | 2 +- WeatherExtension/Services/OpenMeteoService.cs | 25 +++++++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/WeatherExtension/Properties/Resources.Designer.cs b/WeatherExtension/Properties/Resources.Designer.cs index df58c53..0997fa5 100644 --- a/WeatherExtension/Properties/Resources.Designer.cs +++ b/WeatherExtension/Properties/Resources.Designer.cs @@ -60,7 +60,6 @@ internal Resources() { } } - /// /// /// Looks up a localized string similar to Weather API unreachable — may be blocked in your region. /// @@ -106,6 +105,7 @@ public static string connectivity_no_internet { } } + /// /// Looks up a localized string similar to Celsius. /// public static string celsius { diff --git a/WeatherExtension/Services/OpenMeteoService.cs b/WeatherExtension/Services/OpenMeteoService.cs index ca4f938..bb57877 100644 --- a/WeatherExtension/Services/OpenMeteoService.cs +++ b/WeatherExtension/Services/OpenMeteoService.cs @@ -252,14 +252,29 @@ private async Task ProbeConnectivityAsync(string failedEndpoint, CancellationTok var probeResponse = await _httpClient.SendAsync(probeRequest, probeCts.Token).ConfigureAwait(false); - // Probe succeeded — we have internet, so the weather API is specifically unreachable - WeatherLogger.LogToHost( - MessageState.Error, - $"{Resources.connectivity_api_blocked} ({failedEndpoint})"); + if (probeResponse.StatusCode == System.Net.HttpStatusCode.NoContent) + { + // Got expected 204 — we have internet, weather API is specifically unreachable + WeatherLogger.LogToHost( + MessageState.Error, + $"{Resources.connectivity_api_blocked} ({failedEndpoint})"); + } + else + { + // Non-204 response (e.g., captive portal redirect) — treat as no internet + WeatherLogger.LogToHost( + MessageState.Warning, + Resources.connectivity_no_internet); + } + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + // Caller cancelled — don't log, just return + return; } catch { - // Probe failed — no internet connection + // Probe failed (timeout or network error) — no internet connection WeatherLogger.LogToHost( MessageState.Warning, Resources.connectivity_no_internet); From 0a6783772fd5c68e0a524a99fb9fe3c88040031a Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 13:47:50 -0500 Subject: [PATCH 05/10] ci: trigger CI after CS0246 fix The previous run reran at the original SHA before the fix was pushed. This commit triggers a fresh CI run against b114b42 which adds the missing using System.Threading.Tasks directive. From 5e1cca1b4b691a26ed118acce89c3bda47b5138a Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 13:52:29 -0500 Subject: [PATCH 06/10] style: normalize trailing newline in ConnectivityProbeTests.cs Ensures CI picks up the using System.Threading.Tasks fix (b114b42). Previous gh run rerun executed at the old failing SHA. --- WeatherExtension.Tests/ConnectivityProbeTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/WeatherExtension.Tests/ConnectivityProbeTests.cs b/WeatherExtension.Tests/ConnectivityProbeTests.cs index 01f9dde..c87444c 100644 --- a/WeatherExtension.Tests/ConnectivityProbeTests.cs +++ b/WeatherExtension.Tests/ConnectivityProbeTests.cs @@ -123,3 +123,4 @@ await Assert.ThrowsExceptionAsync( Assert.AreEqual(0, callCount, "Cancelled request should not trigger probe"); } } + From af057619eebf3f3bb31a669de70f56041342255f Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 13:56:29 -0500 Subject: [PATCH 07/10] ci: add workflow_dispatch to enable manual CI trigger --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9eb3bd4..2291d05 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,7 @@ name: CI on: + workflow_dispatch: push: branches: [main] pull_request: From 4338b2766e6b4552d3f1b778fe6af90167cdb5a0 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 14:08:15 -0500 Subject: [PATCH 08/10] fix: add missing using directives to ConnectivityProbeTests.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds using System.Net.Http and using System.Threading to resolve CS0246/CS0103 build errors for HttpResponseMessage, CancellationToken, CancellationTokenSource, HttpRequestException, StringContent, and OperationCanceledException. Also reverts workflow_dispatch trigger added as a temporary CI workaround — no longer needed now that all usings are explicit and self-sufficient. --- .github/workflows/ci.yml | 1 - WeatherExtension.Tests/ConnectivityProbeTests.cs | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2291d05..9eb3bd4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,7 +1,6 @@ name: CI on: - workflow_dispatch: push: branches: [main] pull_request: diff --git a/WeatherExtension.Tests/ConnectivityProbeTests.cs b/WeatherExtension.Tests/ConnectivityProbeTests.cs index c87444c..4eff265 100644 --- a/WeatherExtension.Tests/ConnectivityProbeTests.cs +++ b/WeatherExtension.Tests/ConnectivityProbeTests.cs @@ -3,6 +3,8 @@ // See the LICENSE file in the project root for more information. using System.Net; +using System.Net.Http; +using System.Threading; using System.Threading.Tasks; using Microsoft.CmdPal.Ext.Weather.Services; using Microsoft.VisualStudio.TestTools.UnitTesting; From 422b9ff13ba64ce0f8c4e04aed64a0c84cd5dd90 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 14:11:25 -0500 Subject: [PATCH 09/10] fix: add missing using System to ConnectivityProbeTests.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OperationCanceledException is in System namespace — not available without explicit using in this project (no implicit usings configured). --- WeatherExtension.Tests/ConnectivityProbeTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/WeatherExtension.Tests/ConnectivityProbeTests.cs b/WeatherExtension.Tests/ConnectivityProbeTests.cs index 4eff265..4941da9 100644 --- a/WeatherExtension.Tests/ConnectivityProbeTests.cs +++ b/WeatherExtension.Tests/ConnectivityProbeTests.cs @@ -2,6 +2,7 @@ // Bald Bearded Builder LLC licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Net; using System.Net.Http; using System.Threading; From dcbd7e177313d47a0bf3eabad22e81798b80f679 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 14:15:15 -0500 Subject: [PATCH 10/10] fix: accept TaskCanceledException subtype in cancel probe test Assert.ThrowsExceptionAsync checks exact type, but HttpClient wraps OperationCanceledException as TaskCanceledException (a subtype) when propagating through async Task plumbing. Switch to try/catch with catch (OperationCanceledException) to accept any cancellation subtype, which correctly matches the semantic intent of the test. --- WeatherExtension.Tests/ConnectivityProbeTests.cs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/WeatherExtension.Tests/ConnectivityProbeTests.cs b/WeatherExtension.Tests/ConnectivityProbeTests.cs index 4941da9..495ba0d 100644 --- a/WeatherExtension.Tests/ConnectivityProbeTests.cs +++ b/WeatherExtension.Tests/ConnectivityProbeTests.cs @@ -119,9 +119,21 @@ public async Task GetCurrentWeatherAsync_Cancelled_NoProbe() }); using var service = new OpenMeteoService(handler); - await Assert.ThrowsExceptionAsync( - () => service.GetCurrentWeatherAsync(52.52, 13.41, ct: cts.Token)); + // HttpClient wraps OperationCanceledException as TaskCanceledException (a subtype). + // Use try/catch to accept any OperationCanceledException subtype. + Exception? thrown = null; + try + { + await service.GetCurrentWeatherAsync(52.52, 13.41, ct: cts.Token); + Assert.Fail("Expected OperationCanceledException to be thrown"); + } + catch (OperationCanceledException ex) + { + thrown = ex; + } + + Assert.IsNotNull(thrown, "Expected OperationCanceledException (or subtype)"); // Cancelled — probe should not fire Assert.AreEqual(0, callCount, "Cancelled request should not trigger probe"); }