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

🐛 BUG: Can't pipe to wrangler secret put #1303

Closed
williamhorning opened this issue Jun 19, 2022 · 7 comments · Fixed by #1355 or #1336
Closed

🐛 BUG: Can't pipe to wrangler secret put #1303

williamhorning opened this issue Jun 19, 2022 · 7 comments · Fixed by #1355 or #1336
Labels
bug Something that isn't working

Comments

@williamhorning
Copy link
Contributor

What version of Wrangler are you using?

2.0.7 and 2.0.14

What operating system are you using?

Windows and Ubuntu 22.04 on WSL

Describe the Bug

When trying to pipe data from a file to wrangler secret put using cat secret.txt | wrangler secret put SECRET, wrangler returns an error from vadimdemedes/ink with the following text:

Error: Raw mode is not supported on the current process.stdin, which Ink uses as input stream by default.
Read about how to prevent this error on https://github.com/vadimdemedes/ink/#israwmodesupported
    at App.handleSetRawMode (C:\Users\william\AppData\Roaming\nvm\v18.0.0\node_modules\wrangler\wrangler-dist\cli.js:70355:21)
    at C:\Users\william\AppData\Roaming\nvm\v18.0.0\node_modules\wrangler\wrangler-dist\cli.js:70992:9
    at jh (C:\Users\william\AppData\Roaming\nvm\v18.0.0\node_modules\wrangler\wrangler-dist\cli.js:19869:26)
    at exports2.unstable_runWithPriority (C:\Users\william\AppData\Roaming\nvm\v18.0.0\node_modules\wrangler\wrangler-dist\cli.js:16051:16)
    at Pc (C:\Users\william\AppData\Roaming\nvm\v18.0.0\node_modules\wrangler\wrangler-dist\cli.js:16666:16)
    at Qg (C:\Users\william\AppData\Roaming\nvm\v18.0.0\node_modules\wrangler\wrangler-dist\cli.js:19821:18)
    at C:\Users\william\AppData\Roaming\nvm\v18.0.0\node_modules\wrangler\wrangler-dist\cli.js:19811:13
    at V (C:\Users\william\AppData\Roaming\nvm\v18.0.0\node_modules\wrangler\wrangler-dist\cli.js:15977:22)
    at Timeout.w [as _onTimeout] (C:\Users\william\AppData\Roaming\nvm\v18.0.0\node_modules\wrangler\wrangler-dist\cli.js:15818:13)
    at listOnTimeout (node:internal/timers:564:17)

I am able to reproduce this under Windows and Ubuntu 22.04 on WSL in both Wrangler 2.0.7 and Wrangler 2.0.14, in addition to other users in past issues.

@williamhorning williamhorning added the bug Something that isn't working label Jun 19, 2022
@Erisa
Copy link

Erisa commented Jun 20, 2022

What shell/console are you reproducing this under? Anyone know what environment can cause this?

I use Windows 11 with Ubuntu 22.04 in WSL for almost all of my development work and never had this problem (Piped a secret earlier today)
I tried both on Windows itself and inside WSL, with both Windows Terminal and the legacy Console Host, and with both Command Prompt and Powershell 7, all worked completely fine.

@williamhorning
Copy link
Contributor Author

@Erisa I've been able to repro in both the integrated terminal in VSC Insiders (Windows 11 + bash), and Windows Terminal Preview (bash) on both Windows and Ubuntu 22.04 LTS under WSL.

@threepointone
Copy link
Contributor

We have specifically code to workaround this, so I'm surprised to hear of this failure. Could you run these two commands in your terminal for me and tell me what you see?

node -e "console.log(process.stdin.isTTY)"

and

echo 123 | node -e "console.log(process.stdin.isTTY)"

@threepointone
Copy link
Contributor

This is what I see -

➜  ~ node -e "console.log(process.stdin.isTTY)"           
true
➜  ~ echo 123 | node -e "console.log(process.stdin.isTTY)"
undefined
➜  ~ 

@williamhorning
Copy link
Contributor Author

williamhorning commented Jun 20, 2022

@threepointone Running those two commands on Windows (VSC Insiders / Windows Terminal Preview + Bash) returns this:

william@Windows11Laptop MINGW64 ~
$ node -e "console.log(process.stdin.isTTY)"
true

william@Windows11Laptop MINGW64 ~
$ echo 123 | node -e "console.log(process.stdin.isTTY)"
stdin is not a tty

However, running the same commands on Ubuntu 22.04 WSL (Windows Terminal Preview + Bash) returns this:

william@Windows11Laptop:~$ node -e "console.log(process.stdin.isTTY)"
true
william@Windows11Laptop:~$ echo 123 | node -e "console.log(process.stdin.isTTY)"
undefined

@petebacondarwin
Copy link
Contributor

So it seems that in Cygwin/MING64/"Git bash" environments instead of returning undefined or false for process.stdin.isTTY you get an exception thrown with the stdin is not a tty message, right?

I haven't got my Windows machine to hand to test this.

This seems to be a bug in how node is interfacing with the terminal. But if this is the case, we could be a bit more tolerant and change the following function: https://github.com/cloudflare/wrangler2/blob/1d778ae16c432166b39dd6435a4bab49a2248e06/packages/wrangler/src/user/user.tsx#L1131-L1133

To be something like:

function isInteractive(): boolean {
  try {
    return Boolean(process.stdin.isTTY && process.stdout.isTTY);
  } catch {
    return false;
  }
}

@williamhorning
Copy link
Contributor Author

So it seems that in Cygwin/MING64/"Git bash" environments instead of returning undefined or false for process.stdin.isTTY you get an exception thrown with the stdin is not a tty message, right?

Yeah, looking at a few Node.js issues, including nodejs/node#3006, it seems as if Node itself doesn't play well with winpty. However, running the commands in #1303 (comment) while appending .exe (which bypasses winpty) to Node makes Node return undefined instead of stdin is not a tty.

This seems to be a bug in how node is interfacing with the terminal. But if this is the case, we could be a bit more tolerant and change the following function:
...

This code change seems to have worked when testing it out locally on all the environments I've reproduced this issue on.

threepointone pushed a commit that referenced this issue Jun 28, 2022
* fix: Fallback to non-interactive mode on error

If the terminal isn't a TTY, fallback to non-interactive mode instead of throwing an error. This makes it so users of Bash on Windows can pipe to wrangler without an error being thrown.

resolves #1303

Signed-off-by: William Horning <[email protected]>

* Fix formatting and add tests

Signed-off-by: William Horning <[email protected]>

* fixup! Fix formatting and add tests

Co-authored-by: Pete Bacon Darwin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants