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

Adding HTTP status code to error.toJSON #2956

Merged
merged 9 commits into from Sep 5, 2021

Conversation

@byrne-greg
Copy link
Contributor

@byrne-greg byrne-greg commented May 8, 2020

Issue Summary

Previously it was reported in #2947 that the Axios error.toJSON() was not outputting the HTTP status code.

For example;

const axios = require('axios');

axios.get('/404')
  .catch(function (error) {
    console.log(error.toJSON());
  })

would result in the following error message that did not have the HTTP status code of 404 instead only containing the code in the nested in the error message string (such as "Request failed with status code 404").

The Fix

With the following addition to the toJSON function's return object in enhanceError.js:

status: this.response && this.response.status ? this.response.status : null

the response HTTP status code will be added when a response is available. If no response is available (i.e. we can't derive a HTTP status code), we will use null as the value to imply it is intentionally unavailable.

Failing Test without addition

createError.spec.js (existing test-class) (test-amendment)

image

npx jasmine **/*/createError.spec.js
npx: installed 13 in 1.863s
Randomized with seed 49409
Started
F.

Failures:
1) core::createError should create an Error that can be serialized to JSON
  Message:
    Expected undefined to be 200.
  Stack:
    Error: Expected undefined to be 200.
        at <Jasmine>
        at UserContext.<anonymous> (/Users/gregbyrne/Personal_Storage/Personal/axios-fork/test/specs/core/createError.spec.js:26:25)
        at <Jasmine>

enhanceError.spec.js (existing test-class) (new test)

image

npx jasmine **/*/enhanceError.spec.js
npx: installed 13 in 1.481s
Randomized with seed 46485
Started
..F

Failures:
1) core::enhanceError should serialize to JSON with a status of null when there is no response
  Message:
    Expected undefined to equal null.
  Stack:
    Error: Expected undefined to equal null.
        at <Jasmine>
        at UserContext.<anonymous> (/Users/gregbyrne/Personal_Storage/Personal/axios-fork/test/specs/core/enhanceError.spec.js:24:32)
        at <Jasmine>

3 specs, 1 failure
Finished in 0.01 seconds

Passing Test with addition

Execution specifically on the *Error.spec.js tests

npx jasmine **/*/*Error.spec.js 
npx: installed 13 in 1.883s
Randomized with seed 07705
Started
.....
5 specs, 0 failures
Finished in 0.012 seconds

Execution with npm test

Chrome 81.0.4044 (Mac OS X 10.15.4): Executed 211 of 211 SUCCESS (3.457 secs / 3.403 secs)
....................................................
Firefox 75.0.0 (Mac OS X 10.15.0): Executed 211 of 211 SUCCESS (2.179 secs / 2.146 secs)

Side Note: Cannot seem to run these in Safari. When it launches the karma runner, it opens the Safari browser with a Choose file modal already focused to a redirect.html. Also it hangs running on Opera which I don't believe I have installed. When I sigkill, testing moves on to the the bundlesize test

Internal client.html addition

Additionally, I've added a display to the internal client.html which is started using npm start.

This now displays the result of an error in the main display (under an Error div) that uses toJSON() as well as displaying the error object in the console.

No Error

On startup
image

Successful response / no error
image

Error

With a 404 error
image

@byrne-greg
Copy link
Contributor Author

@byrne-greg byrne-greg commented May 11, 2020

After submitting this PR did I realise that there is 123 (at time of writing) open PR's on this project awaiting integration.

Is it safe to say this one likely won't be merged in anytime soon?

Loading

@jasonsaayman jasonsaayman self-requested a review May 12, 2020
@jasonsaayman jasonsaayman added this to In progress in v0.20.0 via automation May 12, 2020
@jasonsaayman jasonsaayman added this to the v0.20.0 milestone May 12, 2020
@jasonsaayman
Copy link
Member

@jasonsaayman jasonsaayman commented May 12, 2020

Hi,

I will review soon. We are trying to release a v0.20 as soon as possible.

Loading

Copy link

@MauricioLoya MauricioLoya left a comment

Good job!

Loading

Copy link
Member

@jasonsaayman jasonsaayman left a comment

Looks good perfect for v0.20.0

Loading

@github-actions
Copy link

@github-actions github-actions bot commented Jun 22, 2020

Hello! 👋 \n\nThis pull request is being automatically marked as stale because it has not been updated in a while. Please confirm that the issue is still present and reproducible. If no updates or new comments are received the pull request will be closed in a few days. \n\nThanks

Loading

@jasonsaayman jasonsaayman moved this from In progress to Done in v0.20.0 Aug 25, 2020
@jasonsaayman jasonsaayman removed this from the v0.20.0 milestone Aug 25, 2020
@atlanteh
Copy link

@atlanteh atlanteh commented Dec 9, 2020

Can this please be merged? We have hacks in multiple places to fix this. thx :)

Loading

@byrne-greg
Copy link
Contributor Author

@byrne-greg byrne-greg commented Jan 8, 2021

@emilyemorehouse @jasonsaayman
do you know if this is on the roadmap for release? it was previously considered for v0.20

Loading

@jasonsaayman jasonsaayman merged commit cd7ff04 into axios:master Sep 5, 2021
3 checks passed
Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v0.20.0
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants