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

newErrorHandling suppresses any error codes #277

Closed
skrud-dt opened this issue Sep 5, 2023 · 4 comments · Fixed by #280
Closed

newErrorHandling suppresses any error codes #277

skrud-dt opened this issue Sep 5, 2023 · 4 comments · Fixed by #280
Assignees
Labels
enhancement Denotes a suggestion or request aimed at improving or adding new features to the project.

Comments

@skrud-dt
Copy link

skrud-dt commented Sep 5, 2023

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 MrRefactoring added the enhancement Denotes a suggestion or request aimed at improving or adding new features to the project. label Sep 6, 2023
@MrRefactoring MrRefactoring self-assigned this Oct 3, 2023
@MrRefactoring
Copy link
Owner

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

@skrud-dt
Copy link
Author

skrud-dt commented Oct 3, 2023

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

@skrud-dt
Copy link
Author

skrud-dt commented Oct 3, 2023

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 added a commit that referenced this issue Oct 5, 2023
@MrRefactoring MrRefactoring changed the title newErrorHandling supresses any error codes newErrorHandling suppresses any error codes Oct 5, 2023
MrRefactoring added a commit that referenced this issue Oct 5, 2023
* [#277]: new error handling additional information added

* [#277]: additional data existing check added

* [#277]: `statusText` added
@MrRefactoring
Copy link
Owner

Released in v2.20.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Denotes a suggestion or request aimed at improving or adding new features to the project.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants