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

Unable to export component with .ttf files #2844

Closed
jimmi-joensson opened this issue Jul 15, 2020 · 16 comments
Closed

Unable to export component with .ttf files #2844

jimmi-joensson opened this issue Jul 15, 2020 · 16 comments
Assignees
Labels

Comments

@jimmi-joensson
Copy link

Describe the bug

A component is not able to export when it is including .ttf files. bit export just gets stuck on "exporting component".
I then tried to change the .ttf files out with .png files and the component exported correctly.

Steps to Reproduce

  1. bit import <component>
  2. Add ./folder/<any-font>.ttf to
  3. bit tag -a
  4. bit export

Expected Behavior

To successfully export the component

Screenshots, exceptions and logs

The export process never procedes from the follwing screenshot:

image

Specifications

@davidfirst
Copy link
Member

@jimmi-joensson , thanks for reporting the issue.

I was trying to reproduce it (without the import and the compiler part, which don't seem to be related to the issue) with no luck.
I suspect that there is something with these specific fonts for some reason. Could you please zip this component with the fonts directory and send it to david@bit.dev ?

@davidfirst davidfirst self-assigned this Jul 15, 2020
@jimmi-joensson
Copy link
Author

@davidfirst I have send you and email with my failling component. Thanks a lot for looking in to it!

@davidfirst
Copy link
Member

@jimmi-joensson , thanks for sending the files! I was able to reproduce it after a while.
This is happening only with node > v14.0.0. No idea why. I'm still investigating it. For now, as a workaround, you can downgrade your node version to 14.0.0 or below.
I'll keep updating this issue with my findings.

@jimmi-joensson
Copy link
Author

@davidfirst Nicely found David 💪 I will downgrade for now, have a great day!

@davidfirst
Copy link
Member

davidfirst commented Jul 17, 2020

After hours of debugging with no clear solution, I'll document my attempts.
The communication between the local and the remote via SSH is done by SSH2 lib, which implemented using streams.
In Bit, the stream communication works in flowing mode and without a mechanism to avoid back-pressure.
This back-pressure is an issue when sending a huge amount of data to the SSH, it causes a high memory consumption and leads to poor performance on export (see #2300 ).
On the client-side, it writes the data to the ssh this way. (src/scope/network/ssh/ssh.ts).

stream.stdin.write(payload);
stream.stdin.end();

On the server-side, it reads the data this way. (src/cli/commands/private-cmds/_put-cmd.ts).

process.stdin
  .on('data', chunk => {
    data += chunk.toString();
  })
  .on('end', () => {
 ...

First, I tried to avoid back-pressure by switching to piping instead of writing directly to the stream.
Converting the string to Readable wasn't easy. Readable.from(payload) caused the string to be sent byte by byte, making the transfer extremely slow. It turned out to be a bug in node <=12.15.0. Finally, I was able to get the Readable working by using a package string-to-stream. With this package, I was able to use pipes as follows:

const readable = stringToStream(payload);
   readable.pipe(stream.stdin).on('error', (pipeErr) => {
      console.log("pipeErr", pipeErr)
});

The chunks I got on the remote, where the correct size - 64K each, and not single bytes.
This probably fixes the backpressure, but sadly, it didn't fix the issue here in this ticket.

Another attempt I made is switching from flowing mode to non-flowing by replacing the data listener with readable on the remote.

.on('readable', () => {
  let chunk;
  while ((chunk = process.stdin.read()) !== null) {
    data += chunk.toString();
  }
})
.on('end', () => {
...

This didn't help as well. Also, adding lower numbers than the highWaterMark in the read() function didn't help.

As of now, I'm pretty much clueless about what the issue is. I know that it hangs during the data transform. It is able to pass part of the data but the "end" event never gets fired, causing the client to hangs forever.

To reproduce it, run the performance e2e-tests with 2,000 components, 50 dependencies each, ssh support, and node version 14.5.0. Keep in mind that 1 out of 10 attempts it does work. Mostly, it hangs while pushing to the remote.

See bug/export-hangs-node-v14 branch for my changes above. (direct link to the commit).

davidfirst added a commit that referenced this issue Jul 17, 2020
…rge data using node v14.1.0 and higher. see #2844 (comment) for more details
@benjamingr
Copy link

@ronag (only if you spot it quickly) any idea what landed in Node 14 that might have caused this?

@ronag
Copy link

ronag commented Jul 19, 2020

Sorry I don't follow what the problem is here. Could you create repro? I don't understand why you would convert the string to stream instead of just writing it.

@davidfirst
Copy link
Member

davidfirst commented Jul 20, 2020

Thanks @benjamingr and @ronag for reading this!
I created a reproducible repo here: https://github.com/davidfirst/stream-issue-node-14-1-0
I added a README file with an explanation of how to reproduce.
If you could take a look, that would be a huge help!

Edit: there is a good chance that the issue is with the SSH2 or the way how I use the streams in regards to the ssh. So I opened an issue in the SSH2 repo as well.

@ronag
Copy link

ronag commented Jul 21, 2020

I suspect this might be related nodejs/node#33076

@ronag
Copy link

ronag commented Jul 21, 2020

Can you reproduce this without involving ssh2?

@davidfirst
Copy link
Member

@ronag , yeah, when I went through the changelog I had the hunch it's related.
I tried to reproduce it without the ssh2 in a few ways, but with no luck. It leads me to believe that it's somewhere in the ssh2 code. I hope my issue there will get attention, if not I'll try to debug ssh2 code and will keep nodejs/node#33076 in mind.
Thanks!

@AlexanderKaran
Copy link

Hi Guys

Just wanted to add I am getting this issue too. Recently upgrade out font styles to come from Theme object and the ttf needs to be in there. Just broke out entire pipeline as I can't push anything new stuff. Anything info I can give to help?

I am going to try and drop my Node version now.

@GiladShoham
Copy link
Member

@davidfirst Can you link the ssh2 issue here, please?

@davidfirst
Copy link
Member

@GiladShoham , it's automatically linked by Github above. Anyway, it's here: mscdex/ssh2#908

@itaymendel
Copy link
Contributor

issue resolved in bit v15

We'll start rolling out v15 of Bit this week, please reach out to me privately on the public slack community for a sneak peek and getting early access to it.

@jimmi-joensson
Copy link
Author

jimmi-joensson commented Mar 25, 2021

@itaymendel sounds great with v15, will give Harmony a spin in the upcoming weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants