Skip to content

Enhancement: improve error messages #2138

@waldekmastykarz

Description

@waldekmastykarz

When API requests fail, we used to print the error message returned by the API, eg. The destination file already exists.. After switching to Axios, we now print the generic web request error, eg. Request failed with status code 400. As you can see, the generic error is meaningless and doesn't tell you what went wrong and how to fix it. We should therefore bring back the old behavior and show the original error as returned by the API.

This fix consists of two parts:

1. Update error processing:

Change:

if (response.error &&
response.error['odata.error'] &&
response.error['odata.error'].message) {
return callback(new CommandError(response.error['odata.error'].message.value));
}

to:

    if (response.isAxiosError &&
      response.response &&
      response.response.data &&
      response.response.data['odata.error'] &&
      response.response.data['odata.error'].message) {
      return callback(new CommandError(response.response.data['odata.error'].message.value));
    }

I assume that because we're using Axios all error response objects we get on failed promises have this shape, but we'd need to double check that.

2. Update all tests

We have a number of tests where we mock the error object using the old structure (response object with an error property). We'd need to find all occurrences and update them to match the new structure (replace error with response.data).

Not sure if we should keep the isAxiosError property to ensure that we're dealing with an Axios error. It gives us an extra level of confidence, but I wonder if we can get non-Axios errors in this code path rendering this check unnecessary.

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions