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

AxiosError's toJSON skips response #4836

Open
KoltesDigital opened this issue Jul 8, 2022 · 6 comments
Open

AxiosError's toJSON skips response #4836

KoltesDigital opened this issue Jul 8, 2022 · 6 comments

Comments

@KoltesDigital
Copy link

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

My projects use Axios, and they always do in a way that that promise rejection will end up being logged by Pino, with the error attached. I noticed that the log message contains the request config, but not the response, which would be helpful to fix the error. I believe that error responses (i.e. whose status is not validated) do not contain actual payload that would flood the log, but should only contain details about the error, therefore it's fine to log it. And, for specific projects where that's not the case, users can still remove the response before the error is dumped, using some feature the logging framework should provide (for Pino one would use custom serializers, perhaps formatters or redact too).

Describe the solution you'd like

Add this.response into AxiosError.toJSON.

I could have made a PR, but I feel there is reason why you haven't done it, so this issue is rather a question about that.

Describe alternatives you've considered

Logging takes place at the API entrypoint (e.g. in an Express middleware) and has no access to the Axios request. Therefore one alternative would be to catch every request as your doc mentions and throw a new error including the response. Obviously that would be factorized in a function so that the code overhead would be low, but it'd still be a burden to think about adding it whenever developers write new code.

Additional context

Thanks for your efforts!

@gustf
Copy link

gustf commented Dec 21, 2022

Hi there, any news regarding this?

@KoltesDigital
Copy link
Author

Yes. I'm reimplementing my services in Rust. It perfectly solves this issue and many other ones.

Maybe actually making the PR would catch the maintainers' attention.

@gustf
Copy link

gustf commented Dec 22, 2022

Sounds like fun rewriting in Rust! 👍

And yes, you are probably right about making a PR

@KoltesDigital
Copy link
Author

Cynicism aside, after I created this ticket, I've experimented with interceptors, and I think they can do the job pretty well. The idea is to transform the error in order to provide a custom serialization. For instance, the following code adds the response's body to the error when serialized:

class AxiosErrorWrapper {
  constructor(error) {
    this.error = error;
  }

  toJSON() {
    return {
      ...this.error.toJSON(),
      responseData: this.error.response && this.error.response.data,
    };
  }
}

axios.interceptors.response.use(undefined, (error) => {
  if (error instanceof AxiosError) {
    return Promise.reject(new AxiosErrorWrapper(error));
  }
  return Promise.reject(error);
});

Logging the full response may be too much: there are lots of internal states and constants (!) that surely aren't worth it.

@meteorlxy
Copy link

For pino custom serializer:

       serializers: {
          err: pino.stdSerializers.wrapErrorSerializer((error) => {
            if (error.name === 'AxiosError') {
              const toJSON = error.toJSON.bind(error);
              error.toJSON = () => ({
                ...toJSON(),
                response: {
                  data: error.response?.data,
                  headers: { ...error.response?.headers },
                },
              });
            }
            return error;
          }),
        }

@ishowta
Copy link

ishowta commented Aug 6, 2023

I checked with git blame and it seems to be to prevent circular references during stringify.

6b44e80

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

No branches or pull requests

4 participants