Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typed API errors #2

Open
simplesmiler opened this issue Jul 21, 2020 · 4 comments
Open

Typed API errors #2

simplesmiler opened this issue Jul 21, 2020 · 4 comments
Labels
enhancement New feature or request
Milestone

Comments

@simplesmiler
Copy link
Owner

Would be nice to have typed API errors, but currently TypeScript does not allow for that.

@simplesmiler simplesmiler added the enhancement New feature or request label Jul 21, 2020
@simplesmiler simplesmiler reopened this Aug 25, 2021
@simplesmiler
Copy link
Owner Author

simplesmiler commented Dec 24, 2021

UPD: I updated the phrasing and example in this comment because it was not clear what is going on.

Can be solved if API errors are not throw but instead returned just like responses, and typed as a union of successful responses and error responses.

Option 1: Use branded version of native Response, and force caller to check status code.

const response = await taxios.post('/tasks', { description: 'Clean the house' });
// response is of type, for example, { status: 201, data: Task } | { status: 409, data: Error }
// and thus data type can be conditionally narrowed by checking status
if (response.status === 201) {
  const task = response.data; // On success data will always be of type Task
}
else {
  const error = response.data; // On failure data will always be of type Error
}

Option 2: Use Result<TOk, TErr> style wrapper.

@simplesmiler
Copy link
Owner Author

simplesmiler commented Aug 29, 2022

@crutch12 and I have talked more about Option 1, and agreed on the following:

  1. taxios.<method> should be typed as returning a discriminated union of possible responses, including error responses, with status code being the discriminator.
    const response = await axios.get('/users', { validateStatus: () => true });
    // response: union of all possible response types, including the error responses
    
    if (response.status === 200) {
      // response: only 200 response
    }
    if (response.status === 201) {
      // response: only 201 response
    }
    if (response.status === 403) {
      // response: only 403 response
    }
  2. Caller is free to use any value of axiosConfig.validateStatus. We do not try to narrow types based on it. In the worst case scenario some response types are inferred as possible, while not actually being reachable in runtime, thus forcing user to check the status code, e.g.:
    try {
      const response = await axios.get('/users');
      // response: union of all possible response types, including the error responses
    
      if (response.status === 200) {
         // response: 200 response
      }
      if (response.status === 201) {
         // response: 201 response
      }
    }
    catch (err) {
      // err: unknown, all type info is lost
    }
  3. To cater to the current try/catch use case, we provide a helper that narrows types of the response following the default axios rule:
    try {
      const response = await axios.get('/users').then(taxios.assertNoError);
      // response: union of all 2xx responses, usually just one
    }
    catch (err) {
      // err: unknown, all type info is lost
    }

Note, that even with this change there are still a reasons to use try/catch, namely unstable network and unreliable server.

@crutch12
Copy link
Collaborator

crutch12 commented Dec 15, 2022

Taxios - обработка статусов

Пример схемы

export namespace Schema {
  export type User = {
    name: string;
  }

  export type PartialUser = Partial<User>

  export type Error404 = {
    message: string
    techMessage?: string;
  }

  export type Error500 = {
    message: string
    techMessage?: string;
    id: number;
  }

  // пример типов для 200, 206, 404, 500
  type UserResponse = {status: 200, data: User} | {status: 206, data: PartialUser} | {status: 404, data: Error404 } | { status: 500, data: Error500 }

  // ...

  // пример схемы
  export type Schema = {
    version: '2',
    routes: {
      '/users/{userId}': {
        // v1:
        // GET: {
        //   params: { userId: number }
        //   response: Schema.User;
        // };

        // v2:
        GET: {
          params: { userId: number }
          response: {
            200: Schema.User,
            206: Schema.PartialUser,
            404: Schema.Error404,
            500: Schema.Error500,
          }
        };
      };
    }
  }
}

Вариант #1 - Не фейлимся при любом статусе (как fetch)

Плюсы:

  • удобно делать switch
  • можно делать left/right проверки

Минусы:

  • противоречит дефолтному поведению axios -> придётся много рефакторить, далеко не все захотят
// пример инициализации
const axios = Axios.create({
  // @NOTE: Говорим axios, что для нас любой http ответ - валиден
  validateStatus: () => true,
  // validateStatus: Taxios.validStatusConfig.validateStatus
})

const taxios = new Taxios<Schema>(axios, {
  validStatus: 'all', // говорим, что все статусы для нас валидны // all | default
  // validStatus: Taxios.validStatusConfig.validStatus
});

// пример вызова
try {
  const response = await taxios.get('/api/user/{userId}', { // UserResponse
    params: { userId: 123 }
    validStatus: 'all', // можем переопределить глобальное (если default), чтобы проще было постепенно рефакторить
    axios: { validateStatus: () => true },
    // ...Taxios.validStatusConfig
  })

  // taxios.$get(...) // data - User | PartialUser | Error404 | Error500

  // left/right (success/fail)
  if (Taxios.isOk(response)) { // response - {status: 200, data: User} | {status: 206, data: PartialUser}
    // ...
  }
  else { // response - {status: 404, data: Error404 } | { status: 500, data: Error500 }
    // ...
  }

  if (response.status === 200) { // response.data - User
    // ...
  }

  if (response.status === 206) { // response.data - PartialUser
    // ...
  }

  if (response.status === 404) { // response.data - Error404
    // ...
  }

  // response.data - Error500
}
catch (err) { // @NOTE: Ошибка сети или фейл по ещё какой-то причине (в данном случае isAxiosError всегда будет false)
  // ...
}

Вариант №2 - Оставляем поведение ошибок как у axios, обрабатываем 400+ через catch

Плюсы:

  • старый код продолжает работать

Минусы:

  • нужен Taxios.isTaxiosError, в котором легко накосячит, если перепутать method/url/config
// пример инициализации
const axios = Axios.create({})

const taxios = new Taxios<Schema>(axios, {
  validStatus: 'default', // говорим, что у нас дефолтное поведение axios
});

const userId = 123;

// пример вызова
try {
  const response = await taxios.get('/api/user/{userId}', { // {status: 200, data: User} | {status: 206, data: PartialUser}
    params: { userId }
  })

  // taxios.$get(...) // data - User | PartialUser

  if (response.status === 200) { // response.data - User
    // ...
  }

  if (response.status === 206) { // response.data - PartialUser
    // ...
  }

  // response.data - never
}
catch (err) { // @NOTE: Ошибка сети или фейл или 400+ // err - any

  if (taxios.isTaxiosError(err, 'GET', '/api/user/{userId}')) { // сигнатура почти как у taxios.url(...)
    // err - AxiosError<{status: 404, data: Error404 } | { status: 500, data: Error500 }>

    if (err.response.status === 404) { // err.response.data - Error404
      // ...
    }

    // response.data - Error500
  }

  if (taxios.isAnyTaxiosError(err)) {
    // response.data - Error500 | Error404 | ... any 400+ response type
  }

}

Вариант #3 - Taxios.assertNoError

Как Вариант #1, но с доп. методом:

try {
  const response = await taxios.get('/users').then(Taxios.assertNoError);
  // response: union of all 2xx responses, usually just one
}
catch (err) {
  // err: unknown, all type info is lost
}

Целевое решение

Я считаю, что нужно поддерживать Вариант #1 и Вариант #2 одновременно. Вариант #3 опционален, его всегда можно прикрутить.

--

@simplesmiler could you check it and add some comments?

@simplesmiler
Copy link
Owner Author

Small suggestions:

  1. I don't really like giving access to both validStatus and validateStatus, because of how fragile it could be. I think it's better to take over the validateStatus within taxios, and only provide validStatus in public API.
  2. The default in validStatus: 'default' will break if we decide to change the default later, I think strict or axios or ok or 2xx will be better.
  3. Rename assertNoError to assertOk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants