2

Consider I am making a pretty generic api call using redux async action. There is an "INIT" dispatch and either "SUCCESS" or "FAILURE", regarding of the result.

Does it make any difference to put the "INIT" dispatch into the try catch block?

OPTION A (outside the try catch block):

export const doSomething = data => async dispatch => {
  dispatch(doSomethingInit()) // <-- outside try block
  try {
    let response = await client.get("/api")
    if (response) {
      dispatch(doSomethingSuccess(response.data))
    }
  } catch (error) {
    dispatch(doSomethingFailure(error))
  }
}

OPTION B (inside the try catch block):

export const doSomething = data => async dispatch => {
  try {
    dispatch(doSomethingInit()) // <-- inside the try block
    let response = await client.get("/api")
    if (response) {
      dispatch(doSomethingSuccess(response.data))
    }
  } catch (error) {
    dispatch(doSomethingFailure(error))
  }
}

I would say it is just a subtle inessential detail, however I do spend a minute thinking about it almost every time I write a new async action… :/

HynekS
  • 2,738
  • 1
  • 19
  • 34
  • I think `dispatch(doSomethingInit())` should never throw an error so better not put it in the try block. You put `await client.get("/api")` in the try block because you **expect** it can throw an error and handle that error in the catch. If you wrote the code without the `async` syntax sugar you'd write: `dispatch(doSomethingInit());client.get("/api").then(happyHandler, errorHandler)` so the first dispatch is not part of what the error handler should handle. – HMR Oct 18 '20 at 19:17
  • @HMR Yes, `dispatch(doSomethingInit())` should never throw – it is used for updating UI and logging. It does make sense as you put it. Thank You. – HynekS Oct 18 '20 at 19:21

1 Answers1

1

I usually avoid thinking that things will not throw.

What if it throws? What code is going to handle it? You might have an outer try-catch block, but now something that happened during doSomething is being handled somewhere else.

I think that since your function/thunk is "trying" to doSomething, it should be responsible for trying and catching anything that might happen during its life cycle. Even if, in theory, it wouldn't happen at that point.

So I usually go with Option B.

export const doSomething = data => async dispatch => {
  try {
    dispatch(doSomethingInit()) // <-- inside the try block
    let response = await client.get("/api")
    if (response) {
      dispatch(doSomethingSuccess(response.data))
    }
  } catch (error) {
    console.error(error);                   // YOU MIGHT LOG IT HERE
    dispatch(doSomethingFailure(error))
  }
}

I also think the code reads better with everything inside the try-catch block.

cbdeveloper
  • 27,898
  • 37
  • 155
  • 336
  • Thank you for your insight. You are probably right that it is always a better choice to put all the function calls in the `try-catch` block (which means no more labour than literally switch those 2 LOC). Therefore, I would rather stick to Option B. – HynekS Oct 19 '20 at 15:38