Upload fails with "Broken Pipe" #335
Comments
|
Not sure why you aren't getting a proper error message but there is 5M limit on upload files. See #91 |
|
Yes, that's size limit issue. asciinema does have error handler for status 413 (which is returned by web server in this case) but it seems it's never properly handled due to how Python's urllib operates. You can read more about the problem here: psf/requests#2422 (comment) |
|
Ran into the same problem today. What do you think about a stopgap measure of emitting a warning client side if the file to be uploaded is over 5M? |
|
I'm regularly running into it well below the 5M limit. That one's only 600K. Plays perfectly locally. |
|
Can reproduce here: Same thing happens on smaller files too. I tried to reproduce with This seems to be an issue specific to |
This should be an FAQ item. As soon as I saw that "broken pipe" repeated twice, I suspected my 6.3M cast exceeded the upload limit. But actually finding this thread took... uhhh... 24 minutes. Around 15 of which I was messing with |
|
I'm getting this error with a small one: Tried uploading it later too: |
|
Maybe the client could stop the upload altogether before it happens. I checked the code and the sizes of the chunks are known in advance before they are uploaded to the server (urllib_http_adapter.py): class MultipartFormdataEncoder:
def __init__(self):
self.boundary = uuid.uuid4().hex
self.content_type = 'multipart/form-data; boundary={}'.format(self.boundary)
@classmethod
def u(cls, s):
if sys.hexversion >= 0x03000000 and isinstance(s, bytes):
s = s.decode('utf-8')
return s
def iter(self, fields, files):
"""
fields is a dict of {name: value} for regular form fields.
files is a dict of {name: (filename, file-type)} for data to be uploaded as files
Yield body's chunk as bytes
"""
encoder = codecs.getencoder('utf-8')
for (key, value) in fields.items():
key = self.u(key)
yield encoder('--{}\r\n'.format(self.boundary))
yield encoder(self.u('Content-Disposition: form-data; name="{}"\r\n').format(key))
yield encoder('\r\n')
if isinstance(value, int) or isinstance(value, float):
value = str(value)
yield encoder(self.u(value))
yield encoder('\r\n')
for (key, filename_and_f) in files.items():
filename, f = filename_and_f
key = self.u(key)
filename = self.u(filename)
yield encoder('--{}\r\n'.format(self.boundary))
yield encoder(self.u('Content-Disposition: form-data; name="{}"; filename="{}"\r\n').format(key, filename))
yield encoder('Content-Type: application/octet-stream\r\n')
yield encoder('\r\n')
data = f.read()
yield (data, len(data))
yield encoder('\r\n')
yield encoder('--{}--\r\n'.format(self.boundary))By the time we know 'yield (data, len(data))' we could thrown an exception and stop the upload because we know it will fail. As a side note, I captured the traffic between my machine and the asciinema website with tcpdump (analized with wireshark), here is the whole conversation until we get dumped because the file is too big: |
|
@josevnz your're right, but it wouldn't be wise to hardcode a serverside config value (the 5M) in the client... IMO what should happen instead, is adequate error reporting. Instead of this:
it should show this:
|
|
Maybe we can check that setting from the server itself (download a file, JSON rest API) so instead of being hard coded on the client is dynamic.
Or as you say we handle the 413 error code differently instead of sending it back with the Nginx error message back to the client.
Sent from Yahoo Mail for iPhone
On Saturday, April 23, 2022, 4:10 AM, Maxim Ivanov ***@***.***> wrote:
@josevnz your're right, but it wouldn't be wise to hardcode a serverside config value (the 5M) in the client...
IMO what should happen instead, is adequate error reporting. Instead of this:
asciinema: upload failed: <urlopen error [Errno 32] Broken pipe>
it should show this:
asciinema: upload failed: server replied 413 Request Entity Too Large
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
I agree with @ulidtko that hardcoding the limit on the client side is a brittle way of solving this. The limit has been changing on the server side over time (2 MB -> 5 MB -> 8 MB), and asciinema recorder distributed in various package repositories isn't always the freshest one (we still have even 8 year old versions uploading recordings, which I'm happy about).
That was desired behavior for a long time, we had this forever: Line 87 in 12b5e52 The problem is psf/requests#2422 (comment) , which breaks this code's expectation. This is deep in Python's stdlib and unlikely to be fixed anytime soon (feel free to read the thread). Anyway, today I've removed the limit configuration for asciinema.org on nginx level but kept default limit of 8 MB in application server's config (Phoenix/Plug/Cowboy). It seems it has fixed the problem of bad client error message 🎉 😅 (while still returning 413, just not from nginx). The limit of 8 MB is still enforced though. Future improvement could be to display current limit in the error message. If we continue relying on 413 then web server (or loadbalancer) could be configured to add extra response header with limit value to the response for all 413 responses, then the client could look for the header and embed it in the error message if present.
That 413 thing has been causing a lot of trouble due to how inadequate Python urllib is with regards to handling premature connection close on 4xx errors (I tried with Python's requests library but it's the same, as it relies on urllib internally). Application-level check/response would potentially workaround this urllib problem. On the other hand HTTP status 413 seems to be the most standard, correct and easy (at least in theory) way of handling upload size limit. I guess we could even combine both - still use 413 but return it from web app's controller instead of from web framework's (or load balancer's) request parser. |
|
The new version of the CLI (the Rust rewrite) handles 413 much better. I tested it with several scenarios and in all cases it properly displays user friendly message. It's not released yet, but it's pretty stable and can be used already (see https://discourse.asciinema.org/t/testing-the-new-rust-version-of-the-asciinema-cli/778/10). I'll see if I can add an additional response header with a value of the current server configured limit. The web server (Phoenix/Cowboy) doesn't let me do it easily so I'll most likely need to do it at the load balancer level (currently using Caddy). If this works out then the CLI could read this header when getting 413 and add the limit value to the error message that is displayed. Speaking of server limit, if you self-host the asciinema server you can now easily change the upload limit: https://docs.asciinema.org/manual/server/self-hosting/configuration/#upload-size-limit |
Here is the app in tmp-folder
The text was updated successfully, but these errors were encountered: