mirror of
https://github.com/asciinema/asciinema.git
synced 2026-04-25 07:55:51 +03:00
[GH-ISSUE #335] Upload fails with "Broken Pipe" #219
Labels
No labels
bug
compatibility
feature request
fit for beginners
help wanted
hosting
idea
improvement
packaging
pull-request
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/asciinema#219
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @TyrfingMjolnir on GitHub (Jan 28, 2019).
Original GitHub issue: https://github.com/asciinema/asciinema/issues/335
Here is the app in tmp-folder
@crazymerlyn commented on GitHub (Jan 28, 2019):
Not sure why you aren't getting a proper error message but there is 5M limit on upload files. See #91
@ku1ik commented on GitHub (Mar 2, 2019):
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: https://github.com/psf/requests/issues/2422#issuecomment-184012765
@emaste commented on GitHub (Oct 29, 2019):
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?
@dankamongmen commented on GitHub (Jan 5, 2020):
I'm regularly running into it well below the 5M limit.
That one's only 600K. Plays perfectly locally.
@nicoonoclaste commented on GitHub (Jan 17, 2020):
Can reproduce here:
Same thing happens on smaller files too. I tried to reproduce with
curlbut this worked fine:This seems to be an issue specific to
asciinema's HTTP API client; I encountered it even when installingasciinemaformpipin a freshvenv; the local python version is 3.7.5 (as packaged in Debian 11/testing)@ulidtko commented on GitHub (Jun 11, 2020):
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
discobotcertification, hoping that'd allow me to post the question in the FAQ discourse section (it didn't).@sontek commented on GitHub (Nov 30, 2020):
I'm getting this error with a small one:
Tried uploading it later too:
@josevnz commented on GitHub (Apr 22, 2022):
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):
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:
@ulidtko commented on GitHub (Apr 23, 2022):
@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:
@josevnz commented on GitHub (Apr 25, 2022):
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: @.***>
@ku1ik commented on GitHub (Jul 2, 2023):
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:
github.com/asciinema/asciinema@12b5e52bb1/asciinema/api.py (L87)The problem is https://github.com/psf/requests/issues/2422#issuecomment-184012765 , 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.
@ku1ik commented on GitHub (Feb 9, 2024):
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
@ku1ik commented on GitHub (Jun 20, 2025):
Since the reported problem was in the previous Python version of the CLI and the new Rust one handles this fine I think this can be closed.