Add status code and error messages to data

luisherranz · 24 January 2020 15:44

Description

Right now Frontity doesn’t return proper status codes for 4xx and 5xx REST API errors. All REST API errors become a 404, which is wrong.

The first step to solve this is to populate data with the correct information from the REST API, so the Frontity app can make decisions based on that info.

User Stories

As a Frontity developer
I want to know about status codes and errors from the REST API
so that I can make decisions of what to show in the front-end

Possible solution

If there is an error in actions.source.fetch we have to populate the data object with info about that error.

REST API errors: When we catch the error, we use that info to populate data.

In order to catch the fetch errors, you need to see if the response was ok and if not, throw the response as the error object:

const response = await fetch("https://httpstat.us/404")
if (!response.ok) { throw response }

Then we add this to data:

      source.data[route] = {
        is404: true, // The status code converted to is.
        errorStatus: 404, // status
        errorStatusText: "Not found" // statusText
        isError: true // always true for errors
        isFetching: false,
        isReady: true
      };

Also, isReady must be true because we already have information about that URL which is the true meaning of isReady.

JavaScript errors: we throw the error again and let the developer deal with it.

luisherranz · 27 January 2020 12:03

I guess you’re going to find out that sometimes you have to throw the response error message (with status and statusText) yourself, like in this case: https://github.com/frontity/frontity/blob/dev/packages/wp-source/src/libraries/handlers/postType.ts#L39

mmczaplinski · 4 February 2020 17:18

@luisherranz

For the react component handling the error logic in the twentytwenty and mars themes, do you have some particular idea in mind?

I think that we are not supposed to show a separate error page .e.g. if there is a 500 error.

I think that we could catch the error and show a “toast notification” or a dismissible dialog. Is that what you had in mind?

luisherranz · 4 February 2020 20:44

My idea was to show a general “Something went wrong” screen. With this type of routing:

      <Main>
        {(data.isFetching && <Loading />) ||
          (data.isArchive && <List />) ||
          (data.isPostType && <Post />) ||
          (data.is404 && <Page404 />) ||
          (data.isError && <ErrorPage />}
      </Main>

Maybe we can reuse ErrorPage for both errors:

      <Main>
        {(data.isFetching && <Loading />) ||
          (data.isArchive && <List />) ||
          (data.isPostType && <Post />) ||
          (data.isError && <ErrorPage />)
      </Main>

and change the message if it is a 404:

const ErrorPage = ({ state }) => {
  const data = state.source.get(state.router.link);
  if (data.is404) return <div>Ops, that page can't be found.</div>;
  return <div>Ops! Something went wrong.</div>;
}

The theme needs to inform the user if either the page doesn’t exist or something else happened.

Of course this is only to show people how it works, they can implement it however they want.

luisherranz · 4 February 2020 22:22

Why do you think that?

mmczaplinski · 4 February 2020 22:43

I think that in the context of a client-side web application it’s a better user experience if we allow the user to stay on the current page if there is a problem with fetching data for the next page, rather than redirect to an “Error Page”.

So, for example:

User is on the archive page.
Clicks on the link to a post The Beauties of Gullfoss
The data fails to load
a) we redirect the user to an error page
b) we show a toast notification saying something went wrong and the user stays on the archive page.

I think b) is better

I’m not sure if this is somehow more difficult to implement but just looking at the code it should not be.

luisherranz · 5 February 2020 11:51

Oh, I see. It could be a good addition but we must provide a) for server-side rendering. I mean, we cannot assume that we are going to always load a page without errors in the first place.

mmczaplinski · 5 February 2020 15:36

Right, I did not take that into account

mmczaplinski · 17 March 2020 00:18

This was implemented in https://github.com/frontity/frontity/pull/291 and https://github.com/frontity/frontity/pull/309

Documentation can be found in https://docs.frontity.org/api-reference-1/wordpress-source#source-get

With the new implementation, if calling get() has returned a status code higher than 400, we add information about the error to the state. For example, if an error code was 500 , the state will include the following properties:

{
  isError: true,
  is500: true,
  errorStatus: 500,
  errorStatusText: "Some string describing the error",

  // booleans that describe the fetch status
  isReady: true,
  isFetching: false
}

Pablo · 18 March 2020 14:02

Thanks for the update @mmczaplinski

@SantosGuillamot We should find a way to mark feature discussions as “done” once they are implemented. I don’t know if it makes sense to close the topic or just edit the title and add a [Released] at the beginning. What do you think?

Reyes · 18 March 2020 15:40

@Pablo you can see the status of a feature here:

But I agree that we should find a way to make it more visible.

SantosGuillamot · 18 March 2020 16:19

I was working on adding the status after the title, precisely to make it more visible as you say. I implemented it but after an update it was breaking the private messages. I have to take a look at it but I didn’t have time yet. This was the initial idea: