MrRefactoring / jira.js

A JavaScript/TypeScript wrapper for the JIRA Cloud, Service Desk and Agile REST API
https://mrrefactoring.github.io/jira.js/
MIT License
349 stars 46 forks source link

newErrorHandling suppresses any error codes #277

Closed skrud-dt closed 9 months ago

skrud-dt commented 10 months ago

Under documentation, we're told to set newErrorHandling: true: https://github.com/MrRefactoring/jira.js#deprecation-warnings

However, this strips out all metadata from the axios error including the status, statusText, and code. I know axios errors include lots of extra information that could cause leaks (like headers, etc.) - but without the status and code fields, it's impossible to know what actually went wrong.

The data field in the Axios Response is maintained, but from JIRA this contains an array of localized strings. Which is impossible to compare against.

Specifically, let's say I try to look up an issue that doesn't exist or that I don't have permission to see.

Using newErrorHandling, this gives me back an object like this:

{
    errorMessages: [
       "Issue does not exist or you do not have permission to see it."
    ]
}

And there is nothing else in the object. This isn't useful to me.

Furthermore, there doesn't seem to be a workaround, because the client's onError middleware receives the transformed error (e.g. the one with all the fields stripped out), not the raw Axios error.

MrRefactoring commented 9 months ago

Hello @skrud-dt! I don't see statusText property in axios errors response. Please clarify which properties you expected in newErrorHandler output?

skrud-dt commented 9 months ago

Hi @MrRefactoring , specifically I wanted these fields:

I ended up setting newErrorHandling: false and then writing some error-handling milddlewhere like this:

      newErrorHandling: false,
      middlewares: {
        onError: <T>(e: T): void {
            if (axios.isAxiosError(e)) {
                e.response = _.pick(e.response, [
                  'status', // The HTTP Response Code, e.g. 400, 404, 401
                  'statusText', // The HTTP Status Text
                  'data', // The data in the response
                ]) as unknown as AxiosResponse;
                e.request = {};
                e.config = {} as unknown as AxiosRequestConfig;
             }
          }
      }

By emptying out the request and config fields I can avoid accidentally leaking things like the Authorization header in the logs.

Indeed it looks like there's no statusText property of AxiosResponse; but it's defined in their TypeScript definition here: https://github.com/axios/axios/blob/v1.x/index.d.ts#L380-L387

skrud-dt commented 9 months ago

Hi @MrRefactoring , specifically I wanted these fields:

  • axiosError.code
  • axiosError.response.statusText
  • axiosError.response.status

I ended up setting newErrorHandling: false and then writing some error-handling milddlewhere like this:

      newErrorHandling: false,
      middlewares: {
        onError: <T>(e: T): void {
            if (axios.isAxiosError(e)) {
                e.response = _.pick(e.response, [
                  'status', // The HTTP Response Code, e.g. 400, 404, 401
                  'statusText', // The HTTP Status Text
                  'data', // The data in the response
                ]) as unknown as AxiosResponse;
                e.request = {};
                e.config = {} as unknown as AxiosRequestConfig;
             }
          }
      }

By emptying out the request and config fields I can avoid accidentally leaking things like the Authorization header in the logs.

Indeed it looks like there's no statusText property of AxiosResponse; but it's defined in their TypeScript definition here: https://github.com/axios/axios/blob/v1.x/index.d.ts#L380-L387

statusText is actually set in the response object here: https://github.com/axios/axios/blob/a48a63ad823fc20e5a6a705f05f09842ca49f48c/lib/adapters/http.js#L510-L516

MrRefactoring commented 9 months ago

Released in v2.20.1