Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Ensure late errors on requests are passed through properly #148

Merged
merged 1 commit into from
Dec 8, 2016

Conversation

iarna
Copy link
Contributor

@iarna iarna commented Dec 8, 2016

Without this patch, if an error is received on the request object AFTER response has been emitted then we retry the request without notifying the response that it failed.

This makes it so that the error is passed along to the response object and leaves any further retries as the responsibility of the caller.

@othiym23
Copy link
Contributor

othiym23 commented Dec 8, 2016

LGTM. 🐑🚀🔥

I'd have to go spelunking in request's commits from a few months ago, but I do think they changed error-handling a few months ago, which is why this has suddenly become an issue.

@coveralls
Copy link

coveralls commented Dec 8, 2016

Coverage Status

Coverage increased (+0.04%) to 90.465% when pulling 533f3dc on late-errors into 2244b9e on master.

@iarna iarna merged commit 5a91aab into master Dec 8, 2016
@iarna iarna removed the in-progress label Dec 8, 2016
@iarna iarna deleted the late-errors branch December 8, 2016 23:59
iarna added a commit to npm/npm that referenced this pull request Dec 9, 2016
Fix a bug in fetch where errors on the request object that came after the
response object had been emitted could result in duplicate callbacks and
suppressed error conditions.  Ultimately this translates to fixing shasum
mismatches in `npm` when they were associated with an `ECONNRESET` or other
network error.

PR-URL: npm/npm-registry-client#139
Fixes: #14626
Credit: @iarna

Add support for sending anonymous cli metrics.

PR-URL: npm/npm-registry-client#148

Fix support for sending anonymous cli metrics.

Credit: @sisidovski
PR-URL: npm/npm-registry-client#147
@zkat zkat mentioned this pull request Jan 26, 2017
4 tasks
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