XKit-Rewritten icon indicating copy to clipboard operation
XKit-Rewritten copied to clipboard

Create and use unified error handler

Open marcustyphoon opened this issue 2 years ago • 3 comments

Description

Problems:

  • Not all of our API requests (apiFetch, megaEdit) are wrapped in defined, consistent error handling code.
  • The friendly apiFetch error messages from exception.body?.errors?.[0]?.detail don't tell the user or us-the-people-reading-the-user's-bug-report what API request actually failed, and the user can't copy them down if they're just in a temporary little notification popup.
  • There is some chance that an API request gives back data that crashes our code and it's hard to tell if that happened or if the API request itself failed.
  • Catching errors can mean they're not in the browser console for analysis of the whole error, not just the part(s) we print.

This:

  • Creates a single error handler that prints both the friendly "detail" text given by a bad API response (whether it is from apiFetch or megaEdit) and the error message detailing what happened (no matter if it is a network failure or a js code exception). It also logs the error to the javascript console, indicating that it's XKit Rewritten so it's easier to distinguish from other stuff in there.
  • Wraps the handler around each location where we call apiFetch or megaEdit a single time due to a user action. (In loops, we already have success/failure counter logic.)
  • ~~Implements a lighter, notify-only handler where we call apiFetch "in the background" (Quick Tags on-post-form-open; inside the Tag Tracking+ main function).~~

This should give us more visibility into e.g. #1271.

Not addressed in this:

  • Tag Tracking+: I did not implement #847
  • Timestamps: ignored; deprecated
  • edit: Instances where we call apiFetch "in the background" (Quick Tags on-post-form-open; inside the Tag Tracking+ main function)

Testing steps

As shown in the reverted commit, I forced every apiFetch to 404 and used each script that is modified here (except mass unliker). I then reverted that and used every script that is modified here (except mass unliker) to confirm basic functionality.

This was not particularly exhaustive. I did try inserting const { a } = null into a function with a showErrorModal handler to confirm that it prints non-network errors.

marcustyphoon avatar Sep 10 '23 08:09 marcustyphoon

  • [x] update for #711

marcustyphoon avatar Sep 22 '23 22:09 marcustyphoon

Something to investigate at some point: what happens if there's a failure attempting to fetch the form key for the mega-edit utility? We can probably throw a slightly more useful error there.

marcustyphoon avatar Sep 23 '23 02:09 marcustyphoon

Logic could be added in the future to give helpful error messages on certain HTTP status codes, like suggesting that the user might be logged out or (if this winds up being a thing) that they might need to log out and log back in/clear cookies/etc.

marcustyphoon avatar Oct 22 '23 17:10 marcustyphoon

(did I mean to do that force push? no. no I did not.)

marcustyphoon avatar Mar 02 '24 18:03 marcustyphoon