Feat: Disable axios behavior

[EinfachHans]

Is your feature request related to a problem? Please describe.

Hey there, it's me again 😊

I currently received 401 error requests from a library i call via axios and mentioned that my api then also returns a 401 (which logs my users out from my app).

I found this: https://tsed.io/docs/controllers.html#axios-response

Describe the solution you'd like

Maybe it would be an option to add a flag to disable this behavior? It could be maybe disabled globally, or per endpoint, what do you think?

Describe alternatives you've considered

No response

Additional context

No response

Acceptance criteria

No response

[Romakita]

Hello @EinfachHans
I don't understand your needs. The code example give you a way to return a response axios from the controller so your are able to handle also the axios response before returns it to the controller. I'm not sure if Ts.ED should provide somethings here, while you are free to wrap response and customize behavior.

See you

[EinfachHans]

@Romakita

What i experience is: In my Controller an AxiosError is thrown, because i do an axios http request in it, which is not catched. The axios error has a status 401. This results in my backend also returning a 401, because it just forwards the axios error.

From the docs:

Ts.ED is able to handle Axios response to wrap it into an Express.js response

This is what i want to disable. I would expect a 500 error in this case

[Romakita]

So use the second example. Catch error and transform the 401 to 500 ;)

[EinfachHans]

I tried to avoid that, because there are many complex situations 🤔😅

[Romakita]

@Get("/")
  async proxy2(@Res() res: Res, @QueryParams("path") path: string): IncomingMessage {
try {
    const response = await Axios.get(`https://cerevoice.s3.amazonaws.com/${path}`, {
      responseType: "stream"
    });

    return response
    } catch (er) {
      // here it's a business rules (or technical rules). 
      if (er.response.status === 401) {
        throw new InternalServerError("My message")
      }
throw er
    }
  }

[Romakita]

For me your needs it's outside of the Ts.ED scope because it's really specific and the result won't be really helpful for other project.

Ts.ED gives a kind of shortcut to create a proxy between express.js and axios (or any other lib that returns the correct interface). If have to do something on the axios response, I encourage you to implement this stuff on the project level (because you can test it, and your code reflect your tech/business rules). Also, maybe changing the status won't be enough and you'll have to change something else on the response error ;)

[EinfachHans]

I think it is in the scope, because that automatic handling of Axios errors is explicit build into tsed.

For me this is a "feature", which i can't disable 🤔

[NachtRitter]

I tried to avoid that, because there are many complex situations 🤔😅

You can try to add response filter which will be handle axios responses.
It will allow you to transform your axios responses in one place.

https://tsed.io/docs/response-filter.html

[EinfachHans]

I currently have a exception filter implemented like this:

@Catch(Error, Exception)
export class ExceptionsFilter implements ExceptionFilterMethods {

which i would expect to run for the AxiosError as well, but this is not the case 🤔

[Romakita]

But why you don't try to do that:

@Get("/")
  async proxy2(@Res() res: Res, @QueryParams("path") path: string): IncomingMessage {
try {
    const response = await Axios.get(`https://cerevoice.s3.amazonaws.com/${path}`, {
      responseType: "stream"
    });

    return response
    } catch (er) {
      // here it's a business rules (or technical rules). 
      if (er.response.status === 401) {
        throw new InternalServerError("My message")
      }
throw er
    }
  }

After that you can implement an interceptor if you want to share this behavior between controllers :) ?

[Romakita]

Here is an interceptor example:

import { Injectable, InterceptorMethods, InterceptorContext} from "@tsed/di";
import { InternalServerError } from "@tsed/exceptions";
import { AxiosError } from "axios";

@Injectable()
export class AxiosInterceptor implements InterceptorMethods {
  async intercept(context: InterceptorContext) {

    try {
      return await context.next()
    } catch (er: unknown) {
      if ("response" in (er as object)) {
        const error = er as AxiosError;

        const wrappedError = new InternalServerError("Internal Server Error",  error.response); //
        wrappedError.body =  error.response?.data;
        wrappedError.headers = error.response?.headers;

        throw wrappedError;
      }

      throw Error
    }
  }
}

[Romakita]

I think it is in the scope, because that automatic handling of Axios errors is explicit build into tsed.

For me this is a "feature", which i can't disable 🤔

Definitively not agree. Ts.ED doesn't handle the axios errors, it just forward any response from http request library that implement the correct interface (see https://github.com/tsedio/tsed/blob/0f514f634d67a2a4492c12dd62a8f28844533a3e/packages/core/src/domain/AnyToPromise.ts).

function isResponse(obj: any) {
  return isObject(obj) && "data" in obj && "headers" in obj && "status" in obj && "statusText" in obj;
}

In the framework there no mention about axios package (in the documentation there is mention of axios). This why I won't implement extra feature to intercept Axios or other Http library error. Another reason is each HTTP Request has his own error format. The framework try to not be coupled with many libraries.

If it's a problem to support response like Axios/Undici/Fetch, I can remove this feature in the next major version ;).

Last point, your will create more complexity on the framework side than it offers advantage and that the existing features already allow you to do what you want :D. If I agree to do what you want, I am certain that it will open the way to other more complex requests because the need will never be complete enough to foresee everything.

See you :)

[EinfachHans]

@Romakita Thank you for your input! I do understand your side 😃

My initial thought was that i want to avoid try/catch, because i have a lot of axios calls at a lot of places and i feel it would be batter to handle errors of them at a single place. An interceptor would be an option, but i would still have to add that interceptor to every endpoint.

Also thank you four the explanation, because of the wording in the docs i thought there is an "extra-axios" solution build into the framework.

I'm still thinking about one question: Wouldn't you say that in this case a exception filter like mine i mentioned above should been called? 🤔 Because at the end of the day there is a unhandled promise rejection of an AxiosError, which inherits Error, so i would expect that to be called

[Romakita]

I'm still thinking about one question: Wouldn't you say that in this case a exception filter like mine i mentioned above should been called? 🤔 Because at the end of the day there is a unhandled promise rejection of an AxiosError, which inherits Error, so i would expect that to be called

Are you sure that the AxiosError is an instance of Error ? Because the exception filter try to resolve that if the prototype chain as Error. Maybe isn't an Error instance :)

[EinfachHans]

Looks like it, yes:

export class AxiosError<T = unknown, D = any> extends Error {

[Romakita]

So if AxioError is exported you can use AxiosError with Catch decorator?

[EinfachHans]

I already tried to explizit add AxiosError into the @Catch, but this doesn't work neither

[Romakita]

I havent idea why isn't intercepted...

[EinfachHans]

I think the logic we are discussing here is the reason 🤔

Here in line 161, it is resolving, not rejecting it:

tsed/packages/core/src/domain/AnyToPromise.ts

Line 161 in 0f514f6

return this.handle(error.response);

Can't this be the reason?

[Romakita]

Arf. Beacause it was easy to forward the axios error response to express without to add a mapper I think ^^.
Keep in mind it's just a simple proxy. So I forward any response to the client. Rejecting axios response will maybe change the final response.

[EinfachHans]

Imo an error thrown should be handled by my exception filter, regardless of which error 🤔

[Romakita]

Maybe but this part is located on core level not on common package.

I'll take a time to see what is the best option:

• deprecate/remove the feature
• find a correct way to manage that without introducing complexity or huge refactor.

Personnaly I prefer to remove the feature ^^

See you

[EinfachHans]

I prefer that as well 👍🏼

Waiting for an update, thank you already! 😊