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

fix: response can be aborted during e.g. fs.stat and hasGzipId12 #250

Closed
wants to merge 1 commit into from

Conversation

ronag
Copy link
Contributor

@ronag ronag commented Apr 13, 2019

Request/Response can be aborted while doing async stuff... handle it.

Would prefer to use isFinished but I've proposed using the on-finished package in another PR so I'll leave the "modern" node way here.

@ronag ronag force-pushed the fix-async-aborted-response branch 2 times, most recently from 2cffc9e to 1d325bd Compare April 13, 2019 10:12
@ronag ronag force-pushed the fix-async-aborted-response branch from 1d325bd to 6853bea Compare April 13, 2019 10:12
@codecov-io
Copy link

codecov-io commented Apr 13, 2019

Codecov Report

Merging #250 into master will decrease coverage by 0.44%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #250      +/-   ##
==========================================
- Coverage   75.31%   74.86%   -0.45%     
==========================================
  Files          11       11              
  Lines         555      557       +2     
  Branches      126      127       +1     
==========================================
- Hits          418      417       -1     
- Misses         48       51       +3     
  Partials       89       89
Impacted Files Coverage Δ
lib/ecstatic.js 71.86% <50%> (-1.07%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0c3e94...6853bea. Read the comment docs.

@ronag
Copy link
Contributor Author

ronag commented Apr 13, 2019

I honestly have no idea how to fix the code coverage on this...

@ronag ronag changed the title fix: response can be aborted during e.g. hasGzipId12 fix: response can be aborted during e.g. fs.stat, and hasGzipId12 Apr 13, 2019
@ronag ronag changed the title fix: response can be aborted during e.g. fs.stat, and hasGzipId12 fix: response can be aborted during e.g. fs.stat and hasGzipId12 Apr 13, 2019
@jfhbrook
Copy link
Owner

I'm honestly not sure how to test this either, these kinda time-dependent things are tough! I say don't sweat the codecov report, I'm not 100% taking it to heart.

@jfhbrook jfhbrook mentioned this pull request Apr 13, 2019
@ronag
Copy link
Contributor Author

ronag commented Apr 13, 2019

I implemented this in the on-finished PR with onFinished.isFinished.

#249

@ronag ronag closed this Apr 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants