site-kit-wp: Implement API error stats collection

Feature Description

When the plugin receives an API error response, we should track those errors for users who have opted into sharing usage stats.

Currently we can only see error rates for certain APIs and we can’t tell which request endpoints are causing the errors. Collecting errors stats will help us monitor, track and report on representative error rate data.


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

Acceptance criteria

  • Whenever an API error occurs, if the user has opted in to tracking, an Analytics tracking event should be sent:
    • Category should be api_error.
    • Name should be a string in the form of {method}:{type}/{identifier}/data/{datapoint} where placeholders represent request arguments by the same names.
    • Label should be {error.message} (code: {error.code}[, reason: {error.data.reason}])
      reason should only be included if present on the error data
    • Value should be the integer status code (use error.data.status if present, otherwise error.code).
  • This tracking should be added for both the legacy API layer and the new googlesitekit-api module.
  • For batch requests in the legacy API layer, errors should be tracked per individual request, not only the entire batch. For example, the API datapoint used for the event name should be the one relevant for the individual request rather than the batch itself.

Implementation Brief

  • Add a centralized error reporting helper method named trackAPIError in a new util/api.js module. The function accepts an error and extracts the code, message and potentially reason, and sends this data off to the analytics tracking helper trackEvent. Note that trackEvent already checks if the user has opted into stats tracking before sending any data.
async function trackAPIError( { method, datapoint, type, identifier, error } ) {
	await trackEvent( 
		'api_error', 
		`${ method }:${ type }/${ identifier }/data/${ datapoint }`, 
		`${ error.message } (code: ${error.code}${ error.data?.reason ? ', reason: ' + error.data.reason : '' } ])`,
		error.data?.status || error.code
	);
}

Data errors are handled in dataAPI.handleWPError and API.siteKitRequest.

  • In assets/js/components/data/index.js: For each call to handleWPError, add method (‘POST’ for set and combinedGet, ‘GET’ for get), datapoint, type and identifier ( in :combinedGet and :get; add a catch clause in set: and add there as well)
  • In dataAPI.handleWPError: Change the accepted parameters: handleWPError( method, datapoint, type, identifier, error ) {

Also, add: trackAPIError( { method, datapoint, type, identifier, error } );

In API.siteKitRequest in assets/js/googlesitekit/api/index.js:

  • Report errors after the catch phrase: } catch ( error ) {:

trackAPIError( { method, datapoint, type, identifier, error } );

Testing

  • Add Jest tests:

In assets/js/googlesitekit/api/index.test.js in the tests in API.get and API.set, add test that verifies the dataLayer method has the correct data pushed to it when an error is returned.

  • Use the same patterns that we already use for testing trackEvent (in assets/js/util/tracking/index.test.js) - using a mock function on the push method of the dataLayer target object.

  • Add new tests for dataAPI in a new file assets/js/components/data/index.test.js:

    • Add tests for get, set and combinedGet (which should test multiple cases: no errors, all errors, one+ errors) similar to described above for API tests.
  • Add a new test file util/api.test.js

    • Add tests for trackAPIEvent following same patterns that we already use for testing trackEvent.

QA Brief

  • To QA this feature, you need to change the reporting account to an Analytics account you have access to. In addition you will need to reproduce error condition to see them reported. We may want to look at introducing a way to mock API error responses using the tester plugin.

Changelog entry

  • Add Analytics tracking events for API request errors.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 30

Most upvoted comments

Great – I think we may want to do the same for handleWPError but we can look at that again in the PR.

Good point - this would be good for consistency.

I saw this was still using positional arguments in the IB – are we still in agreement about using an object instead?

Ah, missed that bit, fixing.

@aaemnnosttv IB updated, thanks for the feedback on the testing approach.