Standardize our approach to handling API errors

[eyeseast]

Imagining an error loading more results for an infinite scroll, I'll propose that if it's our first load we should show a static error message centered in the component. If there's already results and we fail at loading more, we should throw up a toast or snackbar with a message.

Originally posted by @allanlasser in #494 (comment)

There are a bunch of places where we might hit an error related to the API. Right now, there are a bunch of bespoke handlers and lots of places that say something like todo: handle this error better. Let's think of a systematic approach.

[eyeseast]

I use this pattern a lot, and I don't love it:

const resp = await fetch(endpoint, options).catch(console.error);

if (!resp) {
    throw new Error("API unavailable");
}

if (isErrorCode(resp.status)) {
    const { data } = await resp.json();
    throw new Error(data);
}

return resp.json();

The problem is that we can get error info from the API but this isn't a good way to pass it to the frontend.

[eyeseast]

Bigger thinking on error handling: The API will return error codes as a way of telling us we've done something wrong. These aren't actually "errors" in the Javascript sense, though. They shouldn't be raised and caught, in the same way we would if the API is down. This is similar to fetch, which only throws if the request fails outright. A 400 response is still telling us something useful, and we need to pass that information back to the user.

The API is actually pretty good about this, but I've been ignoring it. Here's the relevant line from the docs:

Specifying invalid parameters will generally return a 400 error code with a JSON object with a single "error" key, whose value will be an error message. Specifying an ID that does not exist or that you do not have access to view will return status 404. Trying to create or update a resource you do not have permission to will return status 403.

So we need to treat each 400-type response differently, and we need to do it later in the processing stack. To that end, I made the start of an APIError type in #647:

export interface APIError {
  error: {
    status: number;
    message: string;
  };
}

This probably isn't the final version of this, but it captures my basic thinking. We need to return an object from API methods that has a distinct shape -- we can check for an error property -- and we need to capture the status, message and any fields that a user can fix.

Most of the actual handling will end up in either routes or forms. For 404s, we can use SvelteKit's error(404) function. Maybe also 403. For 400, we need to pass back fields that are invalid and give the user a chance to try again.

[allanlasser]

It makes sense to me and is a great start.

• I agree we try to have our /lib/api methods conform to a consistent interface that includes properties for data and error, but not throwing exceptions that blow up the stack. Something like interface APIResponse<D> and a helper async function getAPIResponse<D>(resp?: Response): APIResponse<D>.
• Relying on logic in /routes to actually handle any error situations also feels correct. It makes sense to call SvelteKit's built-in handlers here, since it will explicitly set the context for if the error is encountered by the server, client, or both.

[eyeseast]

Shopify has a good example of success and error responses: https://shopify.dev/docs/api/admin-graphql#rate_limits

[eyeseast]

Another thing that's tricky about error handling (or maybe makes things less tricky): Some API methods can just fail silently, and we're ok with that. A lot of the account-related methods are like that. If we can't get an authenticated user, we return null and we're in anonymous mode.

Really, this is an issue with forms and permissions. You might try to do something, and if you hit an error, we want to tell you how to fix it. Maybe that limits the scope of what we need to address here.

[sentry-io]

Sentry Issue: DOCUMENTCLOUD-FRONTEND-NEXT-H

[allanlasser]

Another thing that's tricky about error handling (or maybe makes things less tricky): Some API methods can just fail silently, and we're ok with that. A lot of the account-related methods are like that. If we can't get an authenticated user, we return null and we're in anonymous mode.

Really, this is an issue with forms and permissions. You might try to do something, and if you hit an error, we want to tell you how to fix it. Maybe that limits the scope of what we need to address here.

This is something that #704 may also help with—we'll get much better flagging if we assume user isn't null without first checking.