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

Consuming certain HTTPS fetch responses fails with "unexpected end of file" #13058

Open
andreubotella opened this issue Dec 11, 2021 · 18 comments
Labels
bug Something isn't working upstream Changes in upstream are required to solve these issues

Comments

@andreubotella
Copy link
Contributor

andreubotella commented Dec 11, 2021

const res = await fetch("https://wpt.live/resources/testharness.js");
await res.text();
Uncaught Error: request or response body error: error reading a body from connection: unexpected end of file
    at async Object.pull (deno:ext/fetch/26_fetch.js:108:24)

This doesn't fail with every HTTPS response – https://wpt.live/resources/ works, for example – but it seems to be consistent when it does. It doesn't happen with HTTP at all.

This seems to have been introduced in the upgrade to rustls 0.20 in #12488.

@andreubotella andreubotella changed the title Fetching certain HTTPS requests fails with "unexpected end of file" Fetching certain HTTPS URLs fails with "unexpected end of file" Dec 11, 2021
@andreubotella andreubotella changed the title Fetching certain HTTPS URLs fails with "unexpected end of file" Consuming certain HTTPS fetch responses fails with "unexpected end of file" Dec 11, 2021
@andreubotella
Copy link
Contributor Author

This is an upstream bug: rustls/hyper-rustls#162

@bartlomieju bartlomieju added bug Something isn't working upstream Changes in upstream are required to solve these issues labels Dec 12, 2021
@lucacasonato
Copy link
Member

We may need to revert the rustls 0.20 update until we figure this out. The nature of the issue could result in problems outside of WPT.

@bradenmacdonald
Copy link

bradenmacdonald commented Dec 21, 2021

What I assume is the same issue is also causing Deno to fail to cache deps or run type checks on GitHub Actions CI, which has broken my app's builds. For some reason it doesn't seem to happen on my local MacOS Deno, even though it's the same version.

Run deno run ... --unstable --no-check scripts/test-setup.ts
...
Download...
error: error sending request for url (https://deno.land/x/god_crypto@v1.4.9/src/utility/asn1.ts): connection error: unexpected end of file
    at https://deno.land/x/god_crypto@v1.4.9/src/rsa/export_key.ts:4:21
Error: Process completed with exit code 1.

and

Run deno cache --import-map=import_map.json --unstable scripts/test-setup.ts
Download...
error: error sending request for url (https://deno.land/std@0.113.0/streams/conversion.ts): error trying to connect: tls handshake eof
    at file:///home/runner/work/.../core/objstore/objstore.ts:4:42
Error: Process completed with exit code 1.

I have re-run the tests several times and they fail similarly each time, though often on a different file.

This is Deno 1.17.0 running on GitHub Actions default ubuntu runner.

@joao10lima
Copy link

What I assume is the same issue is also causing Deno to fail to cache deps or run type checks on GitHub Actions CI, which has broken my app's builds. For some reason it doesn't seem to happen on my local MacOS Deno, even though it's the same version.

Run deno run ... --unstable --no-check scripts/test-setup.ts
...
Download...
error: error sending request for url (https://deno.land/x/god_crypto@v1.4.9/src/utility/asn1.ts): connection error: unexpected end of file
    at https://deno.land/x/god_crypto@v1.4.9/src/rsa/export_key.ts:4:21
Error: Process completed with exit code 1.

and

Run deno cache --import-map=import_map.json --unstable scripts/test-setup.ts
Download...
error: error sending request for url (https://deno.land/std@0.113.0/streams/conversion.ts): error trying to connect: tls handshake eof
    at file:///home/runner/work/.../core/objstore/objstore.ts:4:42
Error: Process completed with exit code 1.

I have re-run the tests several times and they fail similarly each time, though often on a different file.

This is Deno 1.17.0 running on GitHub Actions default ubuntu runner.

Same here
https://github.com/pipelinit/pipelinit-cli/runs/5235744784?check_suite_focus=true

@c4p-n1ck
Copy link

c4p-n1ck commented May 7, 2022

No fix? not even some monkey-patch?

Like what do I do now? Re-write the entire project in another language? 😕

@Lonniebiz
Copy link

Seriously, other than node.js, does anyone have a work-around?

@djc
Copy link

djc commented Jun 10, 2022

I think one reason this hasn't gotten "fixed" is that it's arguably a bug on the server side. rustls in 0.20 has become more strict about reporting an error if the peer closes its connection without sending a CloseNotify alert, but as I read RFC 5246 and RFC 8446 they do require sending the alert before closing the (write side of) the connection. So it might be interesting to report servers you're seeing this with and figuring out which TLS stack is running on those servers.

@lucacasonato
Copy link
Member

@djc It fails when Deno connects to another Deno server, so rustls on both client and server.

@lucacasonato
Copy link
Member

The other server it is known to fail on is the web-platform-test server which uses Python’s built in TLS, which uses OpenSSL.

@djc
Copy link

djc commented Jun 10, 2022

Well, I feel like there might something specific about Deno because I have not heard from anyone else that's run into similar issues, even though rustls 0.20 has seen significant uptake since it's release 9 months ago.

@lucacasonato
Copy link
Member

@djc That's good info to have. I'll see if I can find anything specific. For client side HTTP, we use reqwest, which uses hyper-rustls under the hood. For server side TLS we use our own rustls<->hyper binding code.

I'll see if I can reproduce the failures when making requests to the WPT test server by just using reqwest.

@djc
Copy link

djc commented Jun 10, 2022

It would be good to check if there are similar issues in the reqwest issue tracker. Also good to focus on the error handling edge cases in the integration between Deno and reqwest on the client side and also (perhaps particularly) on the server side.

@Lonniebiz
Copy link

Lonniebiz commented Jun 10, 2022

Guys, I'm looking for practical solutions that I can implement until this is fixed.

For example, I running deno verison 1.22.2 on this project. Does anyone know which version of Deno precedes this bug? Perhaps I can downgrade to a version of deno where this bug goes away, and later upgrade to the latest deno once it is confirmed fix.

It is my understanding that the reason Deno is throwing this error, is because the web server I'm requesting from isn't always "sending a CloseNotify alert before closing their response stream".

I tire very quickly of frameworks that prioritize correctness over robustness. Something that's not correct is only worthy of a warning if that lack of correctness can be ignored to achieve robustness.

@Lonniebiz
Copy link

Lonniebiz commented Jun 13, 2022

As a workaround, I'm thinking about just retrying the fetch when the bug occurs. Something like this:
https://dev.to/ycmjason/javascript-fetch-retry-upon-failure-3p6g

For me, the bug rarely happens twice in a row, so maybe I can get past it by just retrying x number of times.

Update
This workaround/hack worked great. The bug still occurred 7 times out of 207 fetches, but it never occurred twice in a row in my test. Although I coded it to re-try up to 10 times, each time the bug occurred (those 7 times), on the very next try the fetch succeeded.

All I did to my code was add this function at the top, and then I used fetch_retry() instead of fetch() throughout the entire script.

const fetch_retry = async (url, options, n=10) => {
	let error;
	for (let i = 0; i < n; i++)
	{
		try
		{
			console.log(`Try ${i+1}`);
			return await fetch(url, options);
		}
		catch (err)
		{
			error = err;
		}
	}
  throw error;
};

The function above was written by Jason Yu. The only modifications I made to it, were (1) I made n default to 10 (tries), and (2) I added a console.log() so I could see when the re-tries occurred (oh yeah, and I changed white-space a bit). To see Jason Yu's function without my modifications, go here.

In summary, I consider this a terrible bug that should rank very high on Deno's ToDo list. Fetch is a fundamental utility that should be rock solid in each deno release. However, I'm glad this bug happened: because it caused me to make my own code way more robust (with or without this bug).

@TimDev9492
Copy link

TimDev9492 commented Apr 10, 2023

@Lonniebiz I used this workaround and tried fetching the data 100 times, but the error occurs every single time for me. Are there any other workarounds/fixes available?

@karapigeon
Copy link

@TimDev9492 Did you find a solution?
@Lonniebiz Is there any movement on this issue or any way I can assist? Thanks!

@bombillazo
Copy link

Any solution to this when running deno compile? our deploys are failing due to the deno dependency downloads failing during the compile process.

@RafalKornel
Copy link

RafalKornel commented Mar 21, 2024

Any progress on this issue?

In my small script I am performing some amount of fetches (ex. few thousands)
When I try using await Promise.all(...), then I get +- this result:
image

However, when I change script's logic and try to iterate over promises and await each of them, then all fetches are successfull.
image

image image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream Changes in upstream are required to solve these issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.