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

Spinner crashes CI build because process.stdout.columns === 0 #607

Closed
kubijo opened this issue Apr 8, 2023 · 6 comments · Fixed by #755
Closed

Spinner crashes CI build because process.stdout.columns === 0 #607

kubijo opened this issue Apr 8, 2023 · 6 comments · Fixed by #755
Labels

Comments

@kubijo
Copy link

kubijo commented Apr 8, 2023

Expected Behavior

spinner shouldn't throw errors

Actual Behavior

/tmp/gitlab/builds/xrEFg3V1/0/pool/frontend/.yarn/cache/zx-npm-7.2.1-dc2fa8860f-372e9ef8fc.zip/node_modules/zx/build/goods.js:167
            process.stderr.write(' '.repeat(process.stdout.columns - 1) + '\r');
                                     ^
RangeError: Invalid count value: -1
    at String.repeat (<anonymous>)
    at <anonymous> (/tmp/gitlab/builds/xrEFg3V1/0/pool/frontend/.yarn/cache/zx-npm-7.2.1-dc2fa8860f-372e9ef8fc.zip/node_modules/zx/build/goods.js:167:38)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
Node.js v19.8.1

the offending code:

finally {
    clearInterval(id);
    process.stderr.write(' '.repeat(process.stdout.columns - 1) + '\r');
}

Apparently, the process.stdout.columns is 0 when running in GitLab CI through the NIX script runner & the script utility.

I don't know which part of the stack causes the columns value to be 0, but I suppose script would be the best bet.
I'm using this to force NIX to preserve colors, of my build scripts.

To fix this, I'D propose to Math.max the computed repetition value to 0 at https://github.com/google/zx/blob/main/src/goods.ts#L201

Specifications

  • Version: 7.2.1
  • Platform: Linux
@antonmedv antonmedv added the bug label Apr 8, 2023
@antonmedv
Copy link
Collaborator

Zx should disable spinner in its not tty.

@kubijo
Copy link
Author

kubijo commented Apr 8, 2023

That is perhaps true, but as mentioned in the ticket, it is being run in a pseudo-TTY supplied by script … because there is currently no other way to force NIX not to strip-out ANSI colors

@antonmedv
Copy link
Collaborator

So a check for column is number needed

@kubijo
Copy link
Author

kubijo commented Apr 8, 2023

I suppose that it always will be through some sanity checks / defaults behavior of node, but it will be 0 … So, I would, as mentioned, just clamp the computed number to 0

@digitalkaoz
Copy link

digitalkaoz commented Nov 5, 2024

i disabled the spinner in CI:

import { spinner as googleSpinner } from 'zx'

export async function spinner<T>(fn: () => Promise<T>): Promise<T> {
  if (process.env.CI === 'true') {
    return fn()
  }
  return googleSpinner(fn)
}

@antonmedv is someone interested to add this functionality to zx itself?

@antonmedv
Copy link
Collaborator

I think yes.

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

Successfully merging a pull request may close this issue.

3 participants