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

TypeError on cypress open tty.getWindowSize #1815

Closed
jennifer-shehane opened this issue May 30, 2018 · 11 comments · Fixed by #1828
Closed

TypeError on cypress open tty.getWindowSize #1815

jennifer-shehane opened this issue May 30, 2018 · 11 comments · Fixed by #1828
Assignees
Milestone

Comments

@jennifer-shehane
Copy link
Member

Is this a Feature or Bug?

Bug

Current behavior:

On cypress open, some users are getting TypeError on new update to 3.0.0

TypeError: tty.getWindowSize is not a function
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\node_modules\mocha\lib\reporters\base.js:120:13)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\node_modules\mocha\lib\reporters\base.js:489:3)
at Module._compile (module.js:569:30)
at Object.Module._extensions..js (module.js:580:10)
at Module.load (module.js:503:32)
at tryModuleLoad (module.js:466:12)
at Function.Module._load (module.js:458:3)
at Module.require (module.js:513:17)
at require (internal/module.js:11:18)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\node_modules\mocha\lib\reporters\index.js:3:31)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\node_modules\mocha\lib\reporters\index.js:21:3)
at Module._compile (module.js:569:30)
at Object.Module._extensions..js (module.js:580:10)
at Module.load (module.js:503:32)
at tryModuleLoad (module.js:466:12)
at Function.Module._load (module.js:458:3)
at Module.require (module.js:513:17)
at require (internal/module.js:11:18)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\node_modules\mocha\lib\mocha.js:13:17)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\node_modules\mocha\lib\mocha.js:504:3)
at Module._compile (module.js:569:30)
at Object.Module._extensions..js (module.js:580:10)
at Module.load (module.js:503:32)
at tryModuleLoad (module.js:466:12)
at Function.Module._load (module.js:458:3)
at Module.require (module.js:513:17)
at require (internal/module.js:11:18)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\node_modules\mocha\index.js:3:5)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\node_modules\mocha\index.js:5:3)
at Module._compile (module.js:569:30)
at Object.Module._extensions..js (module.js:580:10)
at Module.load (module.js:503:32)
at tryModuleLoad (module.js:466:12)
at Function.Module._load (module.js:458:3)
at Module.require (module.js:513:17)
at require (internal/module.js:11:18)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\lib\reporter.js:11:11)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\lib\reporter.js:402:4)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\lib\reporter.js:404:3)
at Module._compile (module.js:569:30)
at Object.Module._extensions..js (module.js:580:10)
at Module.load (module.js:503:32)
at tryModuleLoad (module.js:466:12)
at Function.Module._load (module.js:458:3)
at Module.require (module.js:513:17)
at require (internal/module.js:11:18)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\lib\project.js:49:14)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\lib\project.js:654:4)
at Object.<anonymous> (C:\Users\Ladder Digital\AppData\Local\Cypress\Cache\3.0.0\Cypress\resources\app\packages\server\lib\project.js:656:3)
at Module._compile (module.js:569:30)
at Object.Module._extensions..js (module.js:580:10)
at Module.load (module.js:503:32)
at tryModuleLoad (module.js:466:12)
at Function.Module._load (module.js:458:3)
at Module.require (module.js:513:17)
at require (internal/module.js:11:18)
at Object.<anon

Desired behavior:

No TypeError :)

Steps to reproduce:

.node_modules\.bin\cypress open

Versions

Cypress: 3.0.0
OSes observered: Windows 10 Pro, Windows 8.1, Windows 7

@jennifer-shehane
Copy link
Member Author

We recommend downgrading to 2.1.0 in the meantime, while we work on this issue.

npm install --save-dev [email protected]

@gregorybleiker
Copy link

I've had a look at the mocha code at that location. It is referencing a (very) old method of node, that has been removed. Also, the referenced mocha is old (2.4.5), so I wonder if that is the problem...?

@jennifer-shehane jennifer-shehane added the stage: needs investigating Someone from Cypress needs to look at this label May 30, 2018
@kuceb kuceb added stage: ready for work The issue is reproducible and in scope and removed stage: needs investigating Someone from Cypress needs to look at this labels May 30, 2018
@bahmutov
Copy link
Contributor

@brian-mann we really need to bump Mocha, the problem is coming from its base reporter

var tty = require('tty')
// ...
if (isatty) {
  exports.window.width = process.stdout.getWindowSize
      ? process.stdout.getWindowSize(1)[0]
      : tty.getWindowSize()[1];
}

the tty.getWindowSize is super old and it would be a shame to polyfill tty for this

@bahmutov
Copy link
Contributor

Hmm, current Mocha code still has the same logic https://github.com/mochajs/mocha/blob/master/lib/reporters/base.js#L128 - how the hell is it working - it always goes to process.stdout.getWindowSize() path, but for some reason on Windows it goes bad ...

@jennifer-shehane
Copy link
Member Author

I do not believe every Windows user is having this issue. @brian-mann mentioned this maybe having something to do with mocha-multi-reporters

@gregorybleiker
Copy link

gregorybleiker commented May 30, 2018

There is a bug there: glenjamin/mocha-multi#4
However, it is really strange, because as @bahmutov points out, process.stdout.getWindowSize would have to be null for that path to be taken. However, I have checked in my environment and it is definitely not null...
BTW, I tried different node versions (9 and 10) to no avail...

@gregorybleiker
Copy link

gregorybleiker commented May 30, 2018

An other thing that is strange: If I start the cypress.exe by hand (from appdata/local/...) everything works fine... maybe the problem is with how the process is spawn... but then isatty is false, so it won't run that block of code anyway...

@brian-mann
Copy link
Member

Nope, we actually force isatty to be true because we pipe data in order to fix the garbage characters in Windows and we "trick" it into thinking it's being inherit correctly (because it effectively is).

@brian-mann
Copy link
Member

brian-mann commented May 31, 2018

Okay I understand the root cause. When spawning a process in electron in Windows isTTY is always false as per these electron tests...

https://github.com/electron/electron/pull/7143/files#diff-772c67b73618ae46d1b7e8b22e1ec85cR240

What's happening is that we are actually tricking / forcing the electron/node process to believe that windows is in fact a tty. This changed in 3.0 and it's important because we want things like terminal windows + reporters to color text and think we are a tty.

Mocha is using an undocumented method on node to try to figure out the window size, and because we don't monkey patch this it falls through to the second check - which apparently is using a method on tty that has never existed and is a bug in mocha.

To fix this we'll need to polyfill the actual method in node in Windows which is...

function () { 
  return [this.columns, this.rows]
}

HOWEVER! this.columns and this.rows will be undefined in node so we have to polyfill those too! Lucky we use term-size and already have a lib wrapper around that so we can just use that...

brian-mann pushed a commit that referenced this issue May 31, 2018
* server: polyfill tty.getWindowSize close #1815

* correctly polyfill getWindowSize for stdout and stderr, dont touch tty

- use our terminal-size wrapper

* fix not having coffeescript available yet
@pklejnowski
Copy link

Why this bus is closed. Is already fixed?

@jennifer-shehane
Copy link
Member Author

jennifer-shehane commented May 31, 2018

Fixed in 3.0.1.

@jennifer-shehane jennifer-shehane removed stage: ready for work The issue is reproducible and in scope labels Jan 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants