Skip to content

Conversation

@chinesedfan
Copy link
Contributor

Added a test case for #1362.

Copy link
Collaborator

@yasuf yasuf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion for test, nice work!

assert.equal(failure, true, 'request should fail');
assert.equal(error.message, 'maxContentLength size of 2000 exceeded');
done();
}, 100);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can just move the assertions to the catch and exercise the same behavior? to make the specs run faster since people will have to wait the extra 100ms each time to run the test suite

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yasuf Good suggestion. I just copied a similar case and changed a little for redirection. Opened #2699 to do that separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yasuf Can we merge it now?

@chinesedfan chinesedfan changed the title Making sure the error is handled when redirected with too large response Add test for redirecting with too large response Feb 29, 2020
@chinesedfan chinesedfan merged commit 5214445 into axios:master Feb 29, 2020
@chinesedfan chinesedfan deleted the confirm-1362 branch February 29, 2020 11:54
@axios axios locked and limited conversation to collaborators May 3, 2020
@jasonsaayman jasonsaayman added this to the v0.20.0 milestone May 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants