site-kit-wp
site-kit-wp copied to clipboard
`connection-check` endpoint logging errors unnecessarily
Bug Description
Our API client logs request failures automatically. Recently we added a new request for testing if the client is connected to the internet. This is expected to "fail" from time to time and isn't useful to be reported.
This is currently reporting a significant number of events and should be updated to avoid tracking for this endpoint specifically.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Requests to
core/site/data/connection-checkshould not track API errors
Implementation Brief
- [ ] Update
assets/js/util/api.jsadding a new array calledexcludedDataPointsto the top of the file, with the only value being the stringconnection-check.- [ ] Add a new condition
|| excludedDataPoints.includes( datapoint )the first conditional intrackAPIError: https://github.com/google/site-kit-wp/blob/5d3ec528ceb637fbd296861f4ce93e2a6d1d2a46/assets/js/util/api.js#L50-L52
- [ ] Add a new condition
Test Coverage
- [ ] Add a new test
doesn't track excluded datapointsto test the new functionality.
QA Brief
Changelog entry
IB ✔️
QA Update ⚠️
@eugene-manuilov
To test this I compared request on both latest and dev environment.
Q-1) I noticed that once I block request multiple failed request attempt to 'Check - connection' endpoint. Is this expected ?
Dev-
Latest -
Q-2) Regarding the Google Analytics events, no events are being tracked for the failed requests to that endpoint on both the latest and development environments. I have verified this using the Google Analytics debugger. Please let me know if my understanding is incorrect.
@mohitwp
Q-1) I noticed that once I block request multiple failed request attempt to 'Check - connection' endpoint. Is this expected ?
Yes, that is expected because we check whether the internet connection has restored every X seconds.
Q-2) Regarding the Google Analytics events, no events are being tracked for the failed requests to that endpoint on both the latest and development environments. I have verified this using the Google Analytics debugger. Please let me know if my understanding is incorrect.
Oh... just noticed that this blocking approach doesn't help because it generates the fetch_error error which is also ignored. I'll update QAB.
Also I have found another problem in my code that i have fixed and created a new PR for. Moving this task to the CR column for now.
QA Update ⚠️
- Tested on dev environment.
- Compared endpoint on both latest and dev environment.
- @eugene-manuilov Regarding the Google Analytics events, no events are being tracked for the failed requests to that endpoint on both the latest and development environments. I have verified this using the Google Analytics debugger. Is this correct way to verify that ?
- Regarding the Google Analytics events, no events are being tracked for the failed requests to that endpoint on both the latest and development environments. I have verified this using the Google Analytics debugger. Is this correct way to verify that ?
@mohitwp, yeah, looks like we don't track that endpoint even. That happens most likely because we skip all requests that return the fetch_error error code. I think we get the same error code when we are offline 🤔. So, feels like changes that we did in this ticket are not necessary, but it won't hurt to keep them.
Can you try to do the following though: open the SK dashboard of your site, turn off your wifi, wait X minutes and see if you start seeing that you are in the offline mode, then check whether that GA event has been tracked for the connection-check endpoint or not.
@eugene-manuilov I verified this by turning off wifi on both latest and dev environment. No GA event has been tracked for the connection-check endpoint. So, should we keep this code?
Thanks, @mohitwp. Yes, let's keep it anyway.
QA Update ✅
- Tested on dev environment.
- Compared endpoint on both latest and dev environment.
- Regarding the Google Analytics events, no events are being tracked for the failed requests to that endpoint on both the latest and development environments. I have verified this using the Google Analytics debugger.
Aside from a real connection request failure, this can also be raised if the user's WP session expires (prompting WP sign-in modal). In this case, the connection request fails due to a capability check because the user isn't logged in rather than a real network issue, which might be the more likely culprit.