5

I have a promise based API service that requests data from my backend. It also comes with it's own error catch, so I don't have to write it everywhere. When it's written like this:

BackendService.ts

...
getTasks() {
    return axios.get('/api/tasks')
        .then((response) => {
            const {
                tasks
            }: {
                tasks: tasksType
            } = response.data;
            return tasks;
        })
        .catch((error) => console.log(error));
}
...

Entries.tsx

...
const getTasks = () => {
    backendService.getTasks()
        .then((tasks: tasksType) => {
            const filteredTasksData = filterAPIDataForState(tasks);

            addTasks({
                tasks: filteredTasksData
            });
        })
}
...

I get the following error:

TS2345: Argument of type '(tasks: tasksType) => void'
is not assignable to parameter of type '(value: void | tasksType) => void | PromiseLike<void>'.Types of parameters 'tasks'
and 'value'
are incompatible.Type 'void | tasksType'
is not assignable to type 'tasksType'.Type 'void'
is not assignable to type 'TaskInterface[]'.

I guess this is because of the catch, that could make the Promise return nothing (because of the console.log). If I give the getTasks from Entries.tsx it's own catch handler and remove it from BackendService.ts getTasks, it works.

Shouldn't Typescript be able to tell that the .then() in Entries.tsx would not run if there was an error, since there's already a catch handling this situation?

Vincent
  • 137
  • 2
  • 15
  • In *BackendService.ts* you are returning *tasks* variable which is not a promise. If you want to extract the data use *await* or a function inside the *Axios .then()*. See [Promise docs](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise). See the [Axios example](https://github.com/axios/axios#example) and choose which is the most suitable method for you and your project. PS: Since you are using Typescript i suggest you to choose *await*. – Carlo Corradini Jul 12 '20 at 09:16
  • @CarloCorradini from what I see in the [Mozilla page](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then) (see the chaining code), you can return a variable and the following `.then()` will be able to work with it. – Vincent Jul 12 '20 at 09:26
  • Try with [this](https://pastebin.com/nL98Tsrc). @Yousaf as far as I can see there is no promise returned :\ – Carlo Corradini Jul 12 '20 at 09:37
  • @CarloCorradini: That's the way promises work. **ANYTHING** returned inside a `.then()` is wrapped inside a Promise. Similar to anything returned inside a function marked with the `async` keyword returns a promise. Therefore `return tasks` returns a promise of tasks. Please read up on how promises and async/await work – slebetman Jul 12 '20 at 09:57
  • `tasks` may as well be void, so how could you be sure it's of type `taskType`? it works the way it should. – Rafi Henig Jul 12 '20 at 10:07
  • @CarloCorradini your example code is not valid. Try running [this code example](https://pastebin.com/ZDFKnmDf) and see what gets logged on the console. Make sure `axios` is installed before you run this code. Also try [this code example](https://pastebin.com/h0WxsWHc) – Yousaf Jul 12 '20 at 10:54
  • "*It also comes with it's own error catch, so I don't have to write it everywhere.*" - that's not how it works, no. – Bergi Jul 12 '20 at 11:27
  • @RafiHenig That's the whole point of the question, how to avoid/ handle this problem – Vincent Jul 12 '20 at 12:05
  • @Bergi Yeah I am not sure if it works this way, I just thought it would. Is there a way to not write a duplicate catch/ error handler everytime I call the method? – Vincent Jul 12 '20 at 12:06
  • 1
    @Vincent No, there's not. You *must* consider the case that `getTasks()` returns nothing (in case of an error) on every individual call, and handle it appropriately (e.g. showing error messages to the user) or forward the error. If many of these handlers do the same thing, you can of course share code by passing the same function to each `.catch()`. – Bergi Jul 12 '20 at 13:36

2 Answers2

5

the .then() in Entries.tsx would not run if there was an error, since there's already a catch handling this situation?

That is not entirely correct.

catch block in getTasks method in backendService.ts file is returning undefined and when a catch block returns a value instead of:

  • Throwing a value
  • Throwing an error, or
  • Returns a promise that is rejected

instead of invoking the catch block in the calling code, then block is invoked.

This happens because the Promise returned by the getTasks method in the backendService.ts file depends on the following:

  • If the Promise returned by axios.get(...) fulfils then what you do in the then(...) block

  • If the Promise returned by axios.get(...) is rejected then what you do in the catch(...) block

In your case, if the Promise returned by axios.get(...) fulfils, then then(...) block will execute and since it just returns the tasks, Promise returned by getTasks method in backendService.ts file fulfils, leading to the invocation of the then(...) block in the calling code, i.e. in Entries.tsx file.

If the Promise returned by axios.get(...) is rejected, catch(...) block will execute. Since catch(...) block in getTasks method is just logging the error, Promise returned by getTasks method will fulfil with the value of undefined which will lead to the invocation of then(...) block in the calling code, i.e. in Entries.tsx file.

See the following example to understand this.

function getData() {
  // incorrect url which will lead to response.ok being false
  const url = 'https://jsonplaceholder.typicode.com/todo/1';
  return fetch(url)
          .then(response => {
            if (response.ok) {
              return response.json();
            } else {
              throw new Error();
            }
          })
          .catch(error => console.log('catch block in getData function')); 
}

getData()
  .then(data => console.log('then block ran'))
  .catch(error => console.log('error block ran'));

In the above code snippet, as API URL is not correct, response.ok in the then block is false, so error is thrown from the then block which is caught by the catch block in the same function. As this catch block is just logging a message and implicitly returning undefined, Promise returned by the getData function fulfils with the value of undefined. So instead of the catch block, then block executes in the code that calls the getData function.

If you didn't know this then you might be surprised to see this behavior but that is how promises work with catch blocks. The reason for this behavior is that if you have a promise chain that has more than one catch block, like shown below:

fetch(...)
  .then(...)
  .catch(...)   
  .then(...)
  .then(...)
  .catch(...);

then if the first catch block catches an error that is thrown from any of the methods chained before it, then this catch block can do one of the following two things:

  • throw the error which then will lead to the invocation of the next catch block in the promise chain
  • handle the error and return some value

If the first catch block returns normally, the promise returned by catch block will fulfill with that return value of the catch block and this value then becomes the input of the callback function of the next then block in the promise chain. So the promise chain continues instead of stopping as soon as first catch block executes.


Coming back to your code, when the catch block executes in the getTasks method in backendService.ts file, it logs the message and returns undefined which then leads to the invocation of the then block in the Entries.tsx file, instead of the catch block and that is why you get a complaint from typescript about your code.

Solution

You can use one of the following options to solve this problem:

  • Throw the error caught in the catch(...) block of getTasks method in backendService.ts file so that Promise returned by getTasks method is rejected instead of fulfilling with the value of undefined.

  • Remove the catch block in the getTasks function in backendService.ts file and add the catch block in the code that calls getTasks method.

In my opinion, there's no need for a catch block in the backendService.ts file because if the Promise returned by axios.get(...) is rejected, Promise returned by getTasks method will also be rejected if there's no catch block in the getTasks method. So just remove the catch(...) block from the getTasks method and add the catch(...) block where you are calling this getTasks method.

Yousaf
  • 27,861
  • 6
  • 44
  • 69
  • 1
    thank you for your response! I implemented the second solution and just log the error to the console. This is the first time I have seen these multiple catch blocks, but from what you wrote it makes sense to me that it is done this way. – Vincent Jul 12 '20 at 12:35
0

There are many options to handle this

  1. In BackendService.getTasks() just remove .catch() and handle .catch() in the Entries.getTasks()
  2. In BackendService.getTasks() add another then and return with empty tasks for example .then(it => it || [])

Axios won't crash when response error so you must check response properly and handle it as it should be not just destruct response object blindly

  • Hey, thank you for your answer. If I handle it like the first solution, the catch will have to be implemented everytime the backendService.getTasks() gets called. Is there a way to prevent this? Can you maybe explain the reasoning behind the second solution? – Vincent Jul 12 '20 at 09:29
  • The options 2 is just return empty tasks when error. But as I said Axios won't crash when response error so you must check response properly and handle it as it should be not just destruct response object blindly. So you won't need .catch() in the first place. – Woraphot Chokratanasombat Jul 12 '20 at 09:33