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

Carbon closes connection unexpectedly resulting in net::ERR_CONNECTION_CLOSED error #44

Open
lebensterben opened this Issue Feb 17, 2019 · 9 comments

Comments

Projects
None yet
4 participants
@lebensterben
Copy link

lebensterben commented Feb 17, 2019

Hi,

I was trying to process a bash scripts file and the carbon-now-cli always shows that

Error: Sending code to https://carbon.now.sh went wrong

I uploaded my script here, and please try the following command on it.

First, carbon-now colortest, does't work.

There are 75 lines in the file, and changing the the -e option to a value smaller than or equal to 61, it will work.

Then, without modifying the -e, try changing the values for -s option, I find that the CLI would stop working if -s is greater than or equal to 5.

The correspondent code blocks that I suspect to be the cause of the bug are:

Line 4 to line 6:

colortest_usage () {
    echo -e "Usage: colortest [[-v|--verbose] | [-a|--all] | [-h|--help]]\n\nPrint out colors available in this terminal emulator. The default output is a short list of colors. If -v or --verbose is specified, a table containing all combination of background and foreground colors is displayed. Alternatively, if -a or -all is specified, then both short list and the detailed table would be displayed.\n\nNote that there should be no argument to this programme.\n  -v, --verbose\t displays a table with detailed table of combinations of colors.\n  -a, --all    \t displays both brief and detailed results.\n  -h, --help   \t displays this help."
}

And line 62 to line 71:

  while [[ "$#" > 0 ]]; do
    case "$1" in
      -h|--help) colortest_usage;;
      -v|--verbose) colortest_long;;
      -a|--all) colortest_short; colortest_long;;
      --) colortest_short;;
      *) echo "$0: Error(1): This function doesn't accept any argument." >&2 | logger; colortest_usage;unset -f colortest_usage colortest_short colortest_long; option; return 1;;
    esac
    shift
  done

It seems that as long as I'm not asking carbon-now-cli to process both code blocks, then it works fine.

Please take a look.
Thanks

@mixn

This comment has been minimized.

Copy link
Owner

mixn commented Feb 17, 2019

Hi @lebensterben, thanks for the very detailed report on this. 🙂 Also, nice username. 😊

I was able to reproduce this issue for your particular use case and started investigating. Debugged for quite a bit and ended up noticing that it has nothing to do with certain special characters or your file in particular.

It has something to do with the length of the URL that gets produced and Puppeteer tries to open.

The longer the file you’re parsing, the longer the URL (which is ok and completely logical). But seemingly long URLs (above ~6510 characters in total length) lead to a net::ERR_CONNECTION_CLOSED on Puppeteers end. 🤔

Having a foo.txt with 6330x the letter a will fail with this error:

screenshot 2019-02-17 at 22 05 05

…which is the same error I was getting for your file. Or any big file for that matter.

I couldn’t find a Puppeteer issue for long URLs at first glance. Will likely file a report and see what the feedback is. This has a high priority atm because it kind of limits the CLI to small-ish files… and your file wasn’t even that big with 75 LOC.

@Sneezoo

This comment has been minimized.

Copy link

Sneezoo commented Feb 17, 2019

As explained thoroughly in https://stackoverflow.com/a/417184 ,
there is no character limit in URLs in theory, but most servers and browsers defined their own limits.

I guess Puppeteer is not the problem here, but the carbon now server.
As you can see in the link above, puppeteer would likely have it's limits about over 32k.

Also a "connection closed" error sounds more like a carbon issue to me, meaning the server closed the connection unexpectedly.

Update:
Tried it! It's a carbon now issue. If you reload the page after pasting your code in the web app, you will get the same error.

@mixn

This comment has been minimized.

Copy link
Owner

mixn commented Feb 17, 2019

A wild buddy @Sneezoo appears! 😄

https://youtu.be/NrS523dOHU4

@lebensterben

This comment has been minimized.

Copy link
Author

lebensterben commented Feb 17, 2019

@mixn @Sneezoo

Thanks for your quick response. Now I agree that this seems to be related to length limit.

I first tried to run carbon-now on a file with only the two code blocks I suspected to cause the errors, and it went perfectly well.

Then I add comments with Loren ipsum texts and find that it would fail if I have more than 3 paragraphs. I think that proves your theory.

Thanks again.


Also, instead of parsing the whole contents of a file through URL, a more elegant and maybe more secure way is to open the webpage and paste it.

I think this wouldn't be too hard to achieve if there's xclip or pbcopy installed. This could also protect users' privacy because it wouldn't leave anything in the browser history, and the network administrator also won't be able to see the contents of the file.

@mixn mixn changed the title Cannot parse certain special characters correctly net::ERR_CONNECTION_CLOSED due to Carbon server closing connection Feb 26, 2019

@mixn mixn changed the title net::ERR_CONNECTION_CLOSED due to Carbon server closing connection Carbon closes connection unexpectedly resulting in net::ERR_CONNECTION_CLOSED error Feb 26, 2019

@mixn

This comment has been minimized.

Copy link
Owner

mixn commented Feb 26, 2019

@mfix22 I wanted to file an issue for this over in the Carbon repo, but thought I might @ you first to see what your thoughts on it are. :)

@mfix22

This comment has been minimized.

Copy link

mfix22 commented Feb 26, 2019

There is a number of limits outside of Carbon, yes. Browser's have URL length limits which we counteract locally by creating Blob URLs. However, if we need to send the string to Puppeteer (in order to support Safari) we also run Lambda payload size limits (that are non-adjustable at-the-moment).

Does this help at all? I tested the snippet from the original gist and it saved correctly for me. Which browser were you using?

@mixn

This comment has been minimized.

Copy link
Owner

mixn commented Mar 2, 2019

@mfix22 It failed for me on Chrome mobile (after reloading) and gave the error above (see screenshot) when coming from the CLI (hence Puppeteer).

Maybe @Sneezoo could chime in here again as well? :)

@Sneezoo

This comment has been minimized.

Copy link

Sneezoo commented Mar 2, 2019

Chiming in!

Yeah I tested a few things there with the script posted above.
When I just used carbon.now "like everyone else" it worked fine. Like copy-pasting code there and it worked fine. But reloading the page broke it with the previously mentioned connection error on mobile Chrome and Firefox (Laptop; Ubuntu). Also reusing the URL that was generated in the address bar failed in a new tab.

I guess that this happened because of the lambda payload size limits, as @mfix22 suggested.
Especially for the CLI which currently sends the content as one raw string parameter, this limit seems to be a big issue.

So maybe the CLI could eventually restrict the file size, use the the automated copy-paste solution @lebensterben suggested or create/set a blob URL with puppeteer that carbon.now would accept and render?

@mfix22

This comment has been minimized.

Copy link

mfix22 commented Mar 4, 2019

@mixn I'm trying to follow this discussion, but if I miss something and am holding up progress, please open an issue on our repo. I would love to support larger payloads while still using Now 2.0, if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.