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

Always pass on errors if no response #446

Merged
merged 1 commit into from Apr 23, 2018

Conversation

bkeepers
Copy link

When an error occurs that prevents an http response from being generated, the exception is currently obscured by an error message like Cannot read property 'status' of undefined. This is because supertest is attempting to make assertions about the response, but the response is undefined.

This updates the existing error logic, which handles network and system errors, to always return the error and skip response assertions if there is no response.

Fixes #352
Fixes #314
Fixes #370

cc @gr2m @wilhelmklopp

@rbarilani
Copy link

👍

@mikelax mikelax added this to the v3.1.0 milestone Dec 2, 2017
@luislobo
Copy link

luislobo commented Jan 2, 2018

Any news when this will be merged?

@pbalan
Copy link

pbalan commented Jan 22, 2018

+1 Please merge and release

@ddobson
Copy link

ddobson commented Feb 2, 2018

Would love to see this merged.

@mgcrea
Copy link

mgcrea commented Feb 12, 2018

For god sake @mikelax, please merge this...! Thanks!

@SamD
Copy link

SamD commented Feb 21, 2018

How can we get this now, I'm seeing it often

@gr2m
Copy link

gr2m commented Feb 21, 2018

hey folks, be nice to the open source maintainers, I'm sure they do what they can.

In the meanwhile, you can install it directly from github, just set "supertest": "bkeepers/supertest#unknown-errors" in your package.json until the new version is resolved.

If you want to be helpful, you can publish a package like @yournpmusername/supertest-446 to npm so it will be easier to install it for others, then deprecate it on npm once this PR lands.

Don't make assumptions on how easy it is to merge & release, because we don't know. Show some love and patience <3

@cff3
Copy link

cff3 commented Mar 9, 2018

Thanks, @gr2m. Using the version from bkeepers works for me. I can now see where the failures in my code happens.

@kibertoad
Copy link

@mikelax Is there anything I can do to help this land in the master?

@rimiti
Copy link
Contributor

rimiti commented Apr 23, 2018

Thank you everybody for your contributions.
I merge it into next release branch.

@rimiti rimiti self-assigned this Apr 23, 2018
@rimiti rimiti changed the base branch from master to release-v3.1.0 April 23, 2018 09:40
@rimiti rimiti merged commit a28f04c into ladjs:release-v3.1.0 Apr 23, 2018
@bkeepers bkeepers deleted the unknown-errors branch April 23, 2018 15:56
@bkeepers bkeepers restored the unknown-errors branch April 23, 2018 17:05
@rimiti rimiti mentioned this pull request Apr 24, 2018
@bkeepers bkeepers deleted the unknown-errors branch March 30, 2020 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet