feat: Connectivity pre-check on weather API failures by michaeljolley · Pull Request #63 · baldbeardedbuilder/WeatherExtension

michaeljolley · 2026-05-15T11:21:11Z

Summary

Adds a lightweight connectivity probe when weather API calls fail, so we can distinguish "user is offline" from "weather API unreachable/blocked in region".

How it works

On any API failure (non-success HTTP status or exception), sends a HEAD request to connectivitycheck.gstatic.com with a 2-second timeout
If probe succeeds → logs Weather API unreachable — may be blocked in your region (Error level)
If probe fails → logs No internet connection (Warning level)
No user-facing UI changes — pure diagnostic/log signal for triage

Changes

OpenMeteoService.cs: Added ProbeConnectivityAsync helper, called on all 6 failure paths. Added internal constructor for test injection. OperationCanceledException now rethrown instead of swallowed (callers already handle it).
Resources.resx/Designer.cs: 5 new resource strings for diagnostic messages
ConnectivityProbeTests.cs: 6 tests covering probe-on-500, probe-on-exception, no-probe-on-success, no-probe-on-cancel, all 3 endpoints

Risk

Probe adds max 2s latency on failure paths only — no impact on happy path
OperationCanceledException rethrow is a behavior change but all callers already catch it

Closes #58

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>

michaeljolley · 2026-05-15T11:23:52Z

Murdock — xUnit Test Engineer

🔴 One blocking test bug, otherwise LGTM

OpenMeteoService.cs ✅

Probe implementation is correct.

HEAD to connectivitycheck.gstatic.com/generate_204 — correct probe target (Google's captive-portal check, returns 204 on success)
2s timeout via probeCts.CancelAfter(TimeSpan.FromSeconds(2)) linked to caller CT — correct shape
Probe in all six failure paths (non-success status + exception catch) across all three methods ✅
catch (OperationCanceledException) { throw; } before the general catch — correct, no probe on user cancellation ✅

Shared HttpClient — no risk. The probe uses HttpRequestMessage-level headers and a different URL. DefaultRequestHeaders pass through but that just adds User-Agent to the probe, which is harmless. The HttpClient.Timeout is 10s but the probe's 2s comes from CancelAfter on a linked CTS, which cancels the SendAsync call directly — correct.

Minor (non-blocking): The probe adds User-Agent to probeRequest.Headers explicitly, but _httpClient.DefaultRequestHeaders already has it. Duplicate header — harmless but unnecessary. Can delete the probeRequest.Headers.Add line.

Minor (non-blocking): The bare catch in ProbeConnectivityAsync swallows OperationCanceledException if the user's ct fires while the probe is in-flight, logging a false "No internet connection." Since probe logging is diagnostic-only and not user-visible UI, this is acceptable.

ConnectivityProbeTests.cs — 5 of 6 tests are correct

🔴 Blocking: GetCurrentWeatherAsync_Cancelled_NoProbe — wrong callCount assertion

The test pre-cancels the token and expects callCount == 1:

Assert.AreEqual(1, callCount, "Cancelled request should not trigger probe");

But CountingHttpHandler.SendAsync calls cancellationToken.ThrowIfCancellationRequested() before invoking the factory lambda. With a pre-cancelled token, the lambda never runs — callCount stays 0, not 1. This assertion will fail on CI.

Fix: Change to Assert.AreEqual(0, callCount). The test's intent (verify probe did not fire) is correct; only the expected value is wrong. "No probe" means callCount ≤ 1, but with a pre-cancelled token it's actually 0 — asserting 0 is more accurate and still proves the probe didn't fire.

// Was: Assert.AreEqual(1, callCount, "Cancelled request should not trigger probe");
Assert.AreEqual(0, callCount, "Pre-cancelled request: handler never reached, probe should not fire");

Remaining 5 tests ✅

Test	Verdict
GetCurrentWeatherAsync_ApiReturns500_TriggersProbe	✅ callCount == 2 correct
GetCurrentWeatherAsync_ApiThrows_TriggersProbe	✅ callCount == 2 correct
GetForecastAsync_ApiReturns500_TriggersProbe	✅
GetHourlyForecastAsync_ApiReturns500_TriggersProbe	✅
GetCurrentWeatherAsync_ApiSucceeds_NoProbe	✅ callCount == 1, no probe

Non-blocking gap: GetForecastAsync and GetHourlyForecastAsync don't have throw or cancel tests. The logic is structurally identical to GetCurrentWeatherAsync so coverage via that path is acceptable — not a blocker.

Resource strings ✅

Five new strings: connectivity_no_internet, connectivity_api_blocked, connectivity_endpoint_current_weather, connectivity_endpoint_forecast, connectivity_endpoint_hourly_forecast. All present in both .resx and Resources.Designer.cs. Naming is consistent. ✅

One-line fix needed: GetCurrentWeatherAsync_Cancelled_NoProbe — change AreEqual(1, ...)) to AreEqual(0, ...). Everything else is clean. 🚬

Pre-cancelled token throws before handler lambda runs, so callCount is 0, not 1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

michaeljolley · 2026-05-15T11:24:23Z

Team Lead Review — Amy Allen

Murdock's blocker (callCount 0 vs 1 in cancel test) fixed in eebdb4a.

My review:

Architecture: Clean additive change. ProbeConnectivityAsync is private, 2s timeout via linked CTS, HEAD request to well-known neutral endpoint. No impact on happy path.

Safety: OperationCanceledException rethrow is the right call — all callers (WeatherListPage, CurrentWeatherBand, WeatherBandCard) already catch OCE. The old behavior silently returned null, hiding cancellation from callers.

Shared HttpClient: Safe — probe uses per-request headers, timeout via CTS not client-level. Non-blocking note about duplicate User-Agent is cosmetic.

Test coverage: 6 tests cover all paths — 500 status, exception, all 3 endpoints, success (no probe), cancel (no probe). The callCount=0 fix is correct because CountingHttpHandler checks cancellation before invoking the factory.

Non-blocking: The bare catch in ProbeConnectivityAsync swallows OCE during mid-probe cancellation. This is acceptable for a diagnostic-only probe but could be logged if we want more visibility later.

✅ Approved from the team lead.

michaeljolley · 2026-05-15T11:24:42Z

Murdock — re-review after commit eebdb4a

Blocker resolved. ✅

GetCurrentWeatherAsync_Cancelled_NoProbe now asserts Assert.AreEqual(0, callCount, ...) — correct, since CountingHttpHandler fires ThrowIfCancellationRequested before the factory lambda, leaving callCount at 0 when pre-cancelled.

All 6 tests are now correct. PR is clean. 🚬

Copilot

Pull request overview

Adds a lightweight connectivity probe (HEAD to connectivitycheck.gstatic.com/generate_204 with a 2s timeout) on every failure path in OpenMeteoService so logs can distinguish a fully offline user from one whose access to open-meteo is blocked/unreachable. Also introduces an internal test-friendly constructor on OpenMeteoService and adds tests exercising the probe across the three weather endpoints.

Changes:

Add ProbeConnectivityAsync and wire it into all six non-success / exception paths in OpenMeteoService; rethrow OperationCanceledException instead of swallowing it.
Add five new diagnostic resource strings (probe outcome messages + endpoint labels).
Add ConnectivityProbeTests covering probe-on-500, probe-on-exception, probe-on-success (no probe), and cancellation behavior.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File	Description
WeatherExtension/Services/OpenMeteoService.cs	Probe helper + invocation on all failure paths and a test-only constructor
WeatherExtension/Properties/Resources.resx	New diagnostic strings for probe outcomes and endpoint labels
WeatherExtension/Properties/Resources.Designer.cs	Generated accessors for the new strings (hand-edited, with malformed XML doc structure)
WeatherExtension.Tests/ConnectivityProbeTests.cs	New tests for probe behavior across endpoints and on success/cancel paths

Files not reviewed (1)

WeatherExtension/Properties/Resources.Designer.cs: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…esigner.cs - Fix malformed XML doc comments in Resources.Designer.cs (duplicate <summary> before connectivity_api_blocked; missing <summary> 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>

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

WeatherExtension/Properties/Resources.Designer.cs: Language not supported

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.

Ensures CI picks up the using System.Threading.Tasks fix (b114b42). Previous gh run rerun executed at the old failing SHA.

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

WeatherExtension/Properties/Resources.Designer.cs: Language not supported

+
+using System.Net;


michaeljolley · 2026-05-15T18:55:23Z

Temporarily closing to force CI re-trigger on latest commits (CS0246 fix present on branch but CI not triggering for new pushes)

…oth) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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.

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

WeatherExtension/Properties/Resources.Designer.cs: Language not supported

+		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");
+	}


OperationCanceledException is in System namespace — not available without explicit using in this project (no implicit usings configured).

Assert.ThrowsExceptionAsync<T> 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.

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

WeatherExtension/Properties/Resources.Designer.cs: Language not supported

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

Files not reviewed (1)

WeatherExtension/Properties/Resources.Designer.cs: Language not supported

fix: correct expected call count in cancelled probe test

eebdb4a

Pre-cancelled token throws before handler lambda runs, so callCount is 0, not 1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

michaeljolley marked this pull request as ready for review May 15, 2026 11:24

Copilot AI review requested due to automatic review settings May 15, 2026 11:24

Copilot started reviewing on behalf of michaeljolley May 15, 2026 11:25 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

Comment thread WeatherExtension/Properties/Resources.Designer.cs Outdated

Comment thread WeatherExtension/Services/OpenMeteoService.cs

Comment thread WeatherExtension/Services/OpenMeteoService.cs Outdated

root and others added 2 commits May 15, 2026 13:38

Fix CS0246: add missing using System.Threading.Tasks

b114b42

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 15, 2026 18:39

Copilot started reviewing on behalf of michaeljolley May 15, 2026 18:39 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

Comment thread WeatherExtension/Services/OpenMeteoService.cs

root added 2 commits May 15, 2026 13:47

ci: trigger CI after CS0246 fix

0a67837

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.

style: normalize trailing newline in ConnectivityProbeTests.cs

5e1cca1

Ensures CI picks up the using System.Threading.Tasks fix (b114b42). Previous gh run rerun executed at the old failing SHA.

Copilot AI review requested due to automatic review settings May 15, 2026 18:52

Copilot started reviewing on behalf of michaeljolley May 15, 2026 18:52 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

Comment thread WeatherExtension.Tests/ConnectivityProbeTests.cs

Comment on lines +4 to +5

using System.Net;

Comment thread WeatherExtension/Services/OpenMeteoService.cs

Comment thread WeatherExtension/Services/OpenMeteoService.cs

michaeljolley closed this May 15, 2026

michaeljolley reopened this May 15, 2026

root and others added 2 commits May 15, 2026 13:56

ci: add workflow_dispatch to enable manual CI trigger

af05761

Merge main into connectivity-precheck — resolve resx conflict (keep b…

e0e33eb

…oth) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 15, 2026 19:05

Copilot started reviewing on behalf of michaeljolley May 15, 2026 19:05 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

root added 2 commits May 15, 2026 14:11

fix: add missing using System to ConnectivityProbeTests.cs

422b9ff

OperationCanceledException is in System namespace — not available without explicit using in this project (no implicit usings configured).

Copilot AI review requested due to automatic review settings May 15, 2026 19:15

Copilot started reviewing on behalf of michaeljolley May 15, 2026 19:15 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

Comment thread WeatherExtension/Services/OpenMeteoService.cs

Comment thread WeatherExtension/Services/OpenMeteoService.cs

michaeljolley requested a review from Copilot May 15, 2026 19:28

Copilot started reviewing on behalf of michaeljolley May 15, 2026 19:28 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

michaeljolley merged commit 20458cb into main May 15, 2026
4 checks passed

michaeljolley deleted the connectivity-precheck branch May 15, 2026 19:31

This was referenced May 15, 2026

Add GlobalUsings.cs to test project to prevent missing-using build failures #73

Closed

refactor: add GlobalUsings.cs to test project #76

Merged

Uh oh!

Conversation

michaeljolley commented May 15, 2026

Summary

How it works

Changes

Risk

Uh oh!

michaeljolley commented May 15, 2026

🔴 One blocking test bug, otherwise LGTM

OpenMeteoService.cs ✅

ConnectivityProbeTests.cs — 5 of 6 tests are correct

Remaining 5 tests ✅

Resource strings ✅

Uh oh!

michaeljolley commented May 15, 2026

Team Lead Review — Amy Allen

My review:

Uh oh!

michaeljolley commented May 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

michaeljolley commented May 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants