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

getstorybook is failing on Windows #3529

Closed
dahei opened this issue May 4, 2018 · 24 comments
Closed

getstorybook is failing on Windows #3529

dahei opened this issue May 4, 2018 · 24 comments

Comments

@dahei
Copy link

dahei commented May 4, 2018

getstorybook fails on Windows

Simple setup to reproduce:
I've installed latest create-react-app (1.5.2) and ran getstorybook in the freshly created project folder. That's what's happening on Windows:

image

I've tried it several times (also w/ ejecting the cra) - it always fails at this point.

Steps to reproduce

  • Use Windows 10 machine
  • npm install -g create-react-app
  • npm install - g @storybook/cli
  • create-react-app test-app
  • cd test-app
  • getstorybook

Please specify which version of Storybook and optionally any affected addons that you're running

storybook/cli@3.4.3

Affected platforms

  • Windows 10
  • Node 8.3.0
  • NPM 5.3.0

It's running fine on macOS - so it seems to be a Windows specific thing.

@wuweiweiwu
Copy link
Member

Can you try updating storybook/cli to 4.0.0-alpha.4?

@NiroSugir
Copy link

NiroSugir commented May 6, 2018

I'm also experiencing the exact same issue. Updating to 4.0.0-alpha.4 doesn't change anything.

The problem code is found in @storybook/cli/lib/latest_version.js:

    command.stderr.on('data', data => {
      // yarn error
      const info = JSON.parse(data);
      reject(new Error(info.data));
    });

I'm not sure what the underlying issue is, but simply commenting out that code allowed me to install storybook and use it without any issues (so far).

Affected platforms

  • Windows 7
  • Node 10.0.0
  • NPM 5.6.0
  • Yarn 1.6.0

@Keraito
Copy link
Contributor

Keraito commented May 6, 2018

@dahei Do you have yarn installed?
@GeekMode what does yarn info babel-runtime version --json output for you?

@NiroSugir
Copy link

{"type":"inspect","data":"6.26.0"}

@Keraito
Copy link
Contributor

Keraito commented May 6, 2018

@GeekMode Since you already looked into the code, can try changing @storybook/cli/lib/latest_version.js to:

    command.stderr.on('data', data => {
      // yarn error
      process.stdout.write(`${data} \n`);
      // const info = JSON.parse(data);
      // reject(new Error(info.data));
    });

and see what get's logged when you run getstorybook on a fresh create-react-app?

Personally I can't reproduce the error

@NiroSugir
Copy link

NiroSugir commented May 6, 2018

When I stdout, I get blank lines with the following error message:

(node:5612) [DEP0005] DeprecationWarning: Buffer() is deprecated due
to security and usability issues. Please use the Buffer.alloc(), 
Buffer.allocUnsafe(), or Buffer.from() methods instead.

That message has appears anytime I run yarn 1.6.0 with Node 10.

When I console.log(data) instead, I get:

<Buffer 28 6e 6f 64 65 3a 31 34 37 34 34 29 20 5b 44 45 50 30 30 30 35 5d 20 44 65 70 72 65 63 61 74 69 6f 6e 57 61 72 6e 69 6e 67 3a 20 42 75 66 66 65 72 28 ... >

The next line, where that Buffer is passed to JSON.parse, that's where the error appears.

undefined:1
(node:23804) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability  issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
^

SyntaxError: Unexpected token ( in JSON at position 0
    at JSON.parse (<anonymous>)
    at Socket.<anonymous> (K:/sites/story-test/node_modules/@storybook/cli/lib/latest_version.js:40:25)
    at Socket.emit (events.js:182:13)
    at addChunk (_stream_readable.js:280:12)
    at readableAddChunk (_stream_readable.js:265:11)
    at Socket.Readable.push (_stream_readable.js:220:10)
    at Pipe.onread (net.js:638:20)


@wuweiweiwu
Copy link
Member

The error is due to us trying to parse a Buffer as JSON. However, it used to be valid JSON.... maybe something with node 10? A fix would be Buffer.toString()

@DanielRuf
Copy link
Contributor

A fix would be Buffer.toString()

Which is highly recommended. A buffer is not automatically a string and can be anything afaik.

@DanielRuf
Copy link
Contributor

But it is just a warning. The issue is still the SyntaxError: Unexpected token ( in JSON at position 0 which also happens in Node.js 8 as you can see in the initial comment.

@wuweiweiwu
Copy link
Member

wuweiweiwu commented May 7, 2018

Found the bug. spawn doesn't have an encoding option in either node 8 or 10 which is specified here.

we're getting the stdout and stderr which are going to be outputs of npm or yarn which are strings.

The fix would be JSON.parse(data.toString('utf8'))

I'll have to test this on a windows machine tho

@NiroSugir
Copy link

That was my initial reaction as well and that's what I tried, but unfortunately I get the exact same error.

 getstorybook - the simplest way to add a storybook to your project.

 • Detecting project type. ✓
 • Adding storybook support to your "Create React App" based projectundefined:1
(node:8928) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
^

SyntaxError: Unexpected token ( in JSON at position 0
    at JSON.parse (<anonymous>)
    at Socket.<anonymous> (K:/sites/story-test/node_modules/@storybook/cli/lib/latest_version.js:36:25)
    at Socket.emit (events.js:182:13)
    at addChunk (_stream_readable.js:280:12)
    at readableAddChunk (_stream_readable.js:265:11)
    at Socket.Readable.push (_stream_readable.js:220:10)
    at Pipe.onread (net.js:638:20)

@NiroSugir
Copy link

NiroSugir commented May 7, 2018

Finally found the bug. It's actually yarn that is causing the problem. #yarnpkg/yarn#5477. They have it already fixed but are waiting for the next release. Until then, if anyone runs into this issue, just change the following line in @storybook/cli/lib/latest_version.js:

const packageManager = hasYarn() ? 'yarn' : 'npm';

to

const packageManager = 'npm';

Maybe downgrading to a previous release of yarn might do the trick too, but haven't tried that.

@dahei
Copy link
Author

dahei commented May 7, 2018

@GeekMode

Thank's for your investigation effort!

Unfortunately I don't have installed yarn so shouldn't hasYarn() return false? In that case it would have the same effect as your hotfix.

To prove it, I've changed latest_version.js as you suggested, but the error is still the same.

@DanielRuf
Copy link
Contributor

Did it work before (try testing with git bisect)?

@Keraito
Copy link
Contributor

Keraito commented May 7, 2018

So I messed around running getstorybook -f several times in the environment specified by @dahei. So what I noticed was that the process is quite random whether it passes. One time it just goes through eveyrthing without a problem, while the other time it fails as described in the screenshot above.

I then ran the process a few times again while logging the error as I mentioned in #3529 (comment), because the original screenshot seemed like there was additional information being logged. In the end I found this error message by npm (after data:), which matches the original screenshot by @dahei:

image

@wuweiweiwu
Copy link
Member

This is weird :(

@wuweiweiwu wuweiweiwu added the bug label May 8, 2018
@Keraito
Copy link
Contributor

Keraito commented May 8, 2018

So from what I understand from all the comments is that there are two different issues atm, and please correct me if I'm wrong:

  1. The original issue by @dahei, which I could mimic in getstorybook is failing on Windows #3529 (comment), caused by npm since he has no yarn installed.

  2. As specified by @GeekMode, a bug between yarn and node regarding some deprecated Buffer construction, with a matching stacktrace in getstorybook is failing on Windows #3529 (comment). (Which might or might not be related to chore: add Node.js 10 #3536 as said by @wuweiweiwu)

The first issue is something client-side and based on some quick Googling should happen frequently when running any npm command after the first occurrence. Because this has something to do with the environment of the user, storybook could either ignore the message or just display the message during the installation. Something like this:

    command.stderr.on('data', data => {
      // yarn error
      try {
        const info = JSON.parse(data);
        reject(new Error(info.data));
      } catch (e) {
        reject(new Error(data));
      }
    });

I tested this yesterday on my Windows machine and the installation of storybook (through npm) succeeds without any side effects, except that the whole npm update check failed is additionally printed. Might also be good to include a warning message of some kind.

The second problem, from as much as I could understand, is something storybook has no control over and could be considered to be fixed in the same way as the first issue. This is something yarn should take care of on their side, which is on its way. Personally, I think that it doesn't matter whether it is ignored/printed, as long at is doesn't block storybook from installing.

@fralonra
Copy link

Same issue, but the weird thing is that it works fine sometimes without trying any solutions listed above.

The first time I ran getstorybook, it worked. Next time, failed.
I have tried re-install storybook, re-install node modules in the project. None of those can solve the issue.
After several failed attrempts, it worked again, but I don't know why.

  • Windows 10
  • node v8.11.1
  • npm v5.6.0
  • storybook/cli v3.4.3

@DanielRuf
Copy link
Contributor

DanielRuf commented May 10, 2018

Sounds like some race condition or async issue imo.

@Keraito
Copy link
Contributor

Keraito commented May 10, 2018

@fralonra I think this is in line with what I found in #3529 (comment):

So what I noticed was that the process is quite random whether it passes. One time it just goes through everything without a problem, while the other time it fails as described in the screenshot above.

You might be experiencing the same error as displayed in the screenshots in the first comment and in mine, but it's not for sure. There is a fix merged (#3563) that will ignore these warnings. Keep an eye out for it as it's likely some warning coming from your (package manger) side.

@Safa2222
Copy link

@Keraito : after 2 hours of trying to fix the latest_version.js, to update my package.json and try every single tip i found about this isuue it still doesn't work for me :/ any help? does it work at least in an OSX environment ?

@Safa2222
Copy link

Ah it worked now ! i had to try getstorybook many times ! weird !

@Keraito
Copy link
Contributor

Keraito commented May 11, 2018

Hey @Safa2222, the main reason that you're encountering the issue is due to a npm update check failed (see screenshot in #3529 (comment), the top border of the warning box matches the error in your screenshot in #3571). It would be good to see if you encounter this as well when running other npm commands and resolve it if you deem necessary. When I tested this issue on my machines, it was also quite random when the warning would be thrown (exact comment: #3529 (comment)).

To resolve the issue you could either keep running getstorybook in the hope that it succeeds at some point, or your could apply the fix that was merged into storybook but not released yet (which will benefit u in the long run), see https://github.com/storybooks/storybook/pull/3563/files. Hope this can help you!

@Hypnosphi
Copy link
Member

Released as 3.4.5

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

No branches or pull requests

8 participants