Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update periodic connection check for accuracy #9485

Closed
aaemnnosttv opened this issue Oct 9, 2024 · 2 comments
Closed

Update periodic connection check for accuracy #9485

aaemnnosttv opened this issue Oct 9, 2024 · 2 comments
Labels
P2 Low priority Team M Issues for Squad 2 Type: Infrastructure Engineering infrastructure & tooling

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Oct 9, 2024

Feature Description

In #9083 we updated the periodic connection check Site Kit performs which tests for internet connectivity by pinging our REST index endpoint. There is still the potential for inaccurate behavior in the event that the request fails – this happened for me recently when my session timed out and the request failed due to a bad REST nonce. Such a failure is irrelevant to this check though which is only concerned with the request connecting to the backend successfully, rather than authenticating, etc.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The internet connectivity check should only trigger the offline notification if the backend is unreachable

Implementation Brief

  • Update the fetch made to use fetch rather than apiFetch (adapting as necessary) and remove checks on its response (if the call does not throw, consider online=true, otherwise catch and set false
    try {
    const connectionCheckResponse = await apiFetch( {
    path: '/google-site-kit/v1/',
    } );
    // We are only interested if the request was successful, to
    // confirm online status.
    const canReachConnectionCheck = !! connectionCheckResponse;
    setIsOnline( canReachConnectionCheck );
    } catch ( err ) {
    setIsOnline( false );
    }
  • api-fetch cannot be used because it will throw for any non 200-300 response

Test Coverage

  • Update existing coverage as needed

QA Brief

  1. Install tweak extension and add following rule.
   {
    "code": "test_error",
    "message": "Error message.",
    "data": { "reason": "" }
  }
  1. Go to Site Kit dashboard. Disconnect from the internet connection on your system.

  2. Notice that the offline notice should appear.

  3. Re-connect to internet and warning should go away. Make sure that request was intercepted by tweak.

Changelog entry

  • Improve accuracy of periodic network connection check.
@aaemnnosttv aaemnnosttv added P2 Low priority Type: Infrastructure Engineering infrastructure & tooling labels Oct 9, 2024
@eugene-manuilov eugene-manuilov self-assigned this Dec 3, 2024
@eugene-manuilov
Copy link
Collaborator

IB ✔

@eugene-manuilov eugene-manuilov removed their assignment Dec 3, 2024
@ivonac4 ivonac4 added the Team M Issues for Squad 2 label Dec 20, 2024
@ankitrox ankitrox self-assigned this Dec 20, 2024
@ankitrox ankitrox removed their assignment Dec 23, 2024
@ankitrox ankitrox assigned aaemnnosttv and unassigned ankitrox Jan 3, 2025
@aaemnnosttv aaemnnosttv removed their assignment Jan 6, 2025
@wpdarren wpdarren self-assigned this Jan 7, 2025
@wpdarren
Copy link
Collaborator

wpdarren commented Jan 7, 2025

QA Update: ✅

Verified:

  • Following QAB instructions.
  • Noticed that the offline notice should appear.
  • Re-connect to internet and warning goes away. The request was intercepted by tweak.

Image

@wpdarren wpdarren removed their assignment Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Low priority Team M Issues for Squad 2 Type: Infrastructure Engineering infrastructure & tooling
Projects
None yet
Development

No branches or pull requests

6 participants