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

can catch exception when stdin = new Word(fs.readFileSync(0).toString()); failed if current console is not a standard terminal #570

Open
rainydew opened this issue Jun 28, 2023 · 10 comments

Comments

@rainydew
Copy link

rainydew commented Jun 28, 2023

Background: i am using this tool for ci intergation, the ci script is written in Python. I am debugging / testing the ci code in PyCharm

simple code

#!/usr/bin/env python3
# coding: utf-8
import os
os.system("curlconverter example.com")

if i run it in PyCharm, it will error

fs.js:592
  handleErrorFromBinding(ctx);
  ^

Error: EAGAIN: resource temporarily unavailable, read
    at Object.readSync (fs.js:592:3)
    at tryReadSync (fs.js:366:20)
    at Object.readFileSync (fs.js:403:19)
    at file:///usr/local/lib/node_modules/curlconverter/dist/src/cli.js:232:29
    at ModuleJob.run (internal/modules/esm/module_job.js:152:23)
    at async Loader.import (internal/modules/esm/loader.js:166:24)
    at async Object.loadESM (internal/process/esm_loader.js:68:5) {
  errno: -35,
  syscall: 'read',
  code: 'EAGAIN'
}

But when i run a python file in terminal, it runs normally as expected

In Pycharm, for pure script, i can enable Emulate terminal in output console switch in Execution options to avoid receiving this error. (ref https://www.jetbrains.com/help/pycharm/run-debug-configuration.html#run-debug-parameters)

But for some Python service like Django server in PyCharm running, this option is not provided.

This is quite tricky bc I am backend QA (Python & Golang) and not very familiar with Nodejs, I used about 1 hour to figure the actual reason out :(

The source code in cli.js ~line 230 shows

    if (!process.stdin.isTTY) {
        // TODO: what if there's an EOF character? does curl read each @- until EOF?
        stdin = new Word(fs.readFileSync(0).toString());
    }

If the stdin is not tty (standard terminal), curlconverter will try to read from StdIn pipe
But in PyCharm, of course, it only enable StdIn pipe when it needs users to input for intraction
So the code will always fail

In fact, the stdin pipe sometimes is not nessary, esp in ci procedure, can we add try-catch block here, if the code failed, we don't need to break the usage?

@verhovsky
Copy link
Member

Makes sense. The idea here is to support generating code for a command like this:

echo some data | curlconverter --data @- example.com

while also supporting piping a whole curl command from stdin

echo curl example.com | curlconverter -

The command curl --data @- example.com tells curl to read data to send from stdin, so to generate similar code we also need to read from stdin if there's any data on it.

I was told that isTTY is the way to check if there's a stdin to read data from

https://stackoverflow.com/a/39801858

but looks like it's not that simple. Wrapping this in a try block makes sense, I can do it in a few days or feel free to submit a PR. The toString should be separate from the try

@verhovsky
Copy link
Member

verhovsky commented Jun 29, 2023

Actually maybe I just messed up and it just needs to be the opposite

if (process.stdin.isTTY) {

But also, why are you running curlconverter from the command line? You might be better off piping the command from stdin as I explain here

#322 (comment)

or using it from Node.js. Are you also trying to create pycharm extension? Maybe we should make an official one like we have for VSCode if you're the second person to ask for it. It just seems so complicated.

PS. adding # coding: utf-8 to all your files isn't necessary with Python 3. It's a Python 2 thing https://stackoverflow.com/questions/14083111/should-i-use-encoding-declaration-in-python-3

@rainydew
Copy link
Author

rainydew commented Jun 29, 2023

i just developed a service for QAs and Devs, in that service, we use other tools to capture from network flow or generate from API document platforms, to generate curl commands for testing propose. But curl commands is not easy to intergrate with other test frameworks, and i found your tool works this thing perfectly so we intergrate your tool into our service.
Our service is written in Python, which calls your command line tool via subprocesss.Popen method in our Python code (python backend run a shell to execute your command tool in a child process, and then collect the output from the child process)

We met this issue because we develop our service in Python via Pycharm IDE, and we found when we run our service in Linux container (for live environment usage), all functions worked normally. But if we want to try running our code in Pycharm, e.g. for unit test our code or scripts for self testing, we met this issue because normally Pycharm IDE doesn't provide a standard terminal for debug / test code execution. This effects the development experience, luckily i found it why and can avoid by doing configurations in IDE (but not all cases, we currently use Bottle framework and it's ok. But if in Django and we want to use Pycharm Django intergration functions then this switch is not available, we cannot enforce a simulated real terminal in that case).

@rainydew
Copy link
Author

and

PS. adding # coding: utf-8 to all your files isn't necessary with Python 3. It's a Python 2 thing

yes, we are adding this just bc we migrated some code from Python 2 about 1 year ago

@verhovsky
Copy link
Member

I looked more into this and it's pretty complicated. The current check if (!process.stdin.isTTY) { is correct. I'm not sure that it'll help to wrap it in a try block, because in my experiments it just hangs trying to read from stdin indefinitely. I never saw the EAGAIN error testing it like this:

node -e "console.log(fs.readFileSync(0)); process.exit(1)"
echo | node -e "console.log(fs.readFileSync(0)); process.exit(1)"
npx nodemon --exec 'node -e "console.log(fs.readFileSync(0)); process.exit(1)"'

The last option hangs. Are you getting this error on Windows or Linux or macOS?

If this is too hard let's just remove this code because obviously supporting people like you using curlconverter from a strange command line environment is more important than someone piping data over stdin.

The correct way to do this would be to read stdin whenever we actually see a command line argument that requires reading from stdin like --data @- but that's way harder.

@rainydew
Copy link
Author

rainydew commented Jul 4, 2023

Are you getting this error on Windows or Linux or macOS?

i got the same issue both in MacOS & Windows Pycharm, Windows Git Bash and Windows PowerShell ISE
can check by this
echo "import os; print(os.isatty(0))" | python

@rainydew
Copy link
Author

rainydew commented Jul 4, 2023

I looked more into this and it's pretty complicated. The current check if (!process.stdin.isTTY) { is correct. I'm not sure that it'll help to wrap it in a try block, because in my experiments it just hangs trying to read from stdin indefinitely. I never saw the EAGAIN error testing it like this:

node -e "console.log(fs.readFileSync(0)); process.exit(1)"
echo | node -e "console.log(fs.readFileSync(0)); process.exit(1)"
npx nodemon --exec 'node -e "console.log(fs.readFileSync(0)); process.exit(1)"'

The last option hangs. Are you getting this error on Windows or Linux or macOS?

If this is too hard let's just remove this code because obviously supporting people like you using curlconverter from a strange command line environment is more important than someone piping data over stdin.

The correct way to do this would be to read stdin whenever we actually see a command line argument that requires reading from stdin like --data @- but that's way harder.

can we only enable this check when - arguement is placed in curlconveter? since only that time we need to read buffers from stdin...

e.g. curlconverter example.com obviously no need to take input from stdin, and no need to check; otherwise, curlconverter - or curlconverter --stdin needs, we then enable it?

@rainydew
Copy link
Author

rainydew commented Jul 4, 2023

test result:

node -e "console.log(fs.readFileSync(0)); process.exit(1)"
echo | node -e "console.log(fs.readFileSync(0)); process.exit(1)"
npx nodemon --exec 'node -e "console.log(fs.readFileSync(0)); process.exit(1)"'

1st cmd will hang in Pycharm (and also Git Bash / PowerShell ISE too), even after ctrl+D, it will still ignore all inputs and the only way is to force kill the process
in Mac Terminal it runs as expected, wait for all input and return after CTRL+D

2nd cmd will return normally with 0a input in both standard and non-standard terminals

3rd cmd runs like 1st cmd, hang in Pycharm and work in terminal (always reading and display)

@verhovsky
Copy link
Member

can we only enable this check when - arguement is placed

No, it's the opposite. This code only runs when there's no - argument to curlconverter. If you pass - that read happens here:

const input = fs.readFileSync(0, "utf8");

The idea of the code you're getting the error from is that you can have a curl command like this:

curl --data @- example.com

which means "read data from stdin and send it to example.com", so you could have a command like this

echo "hello world" | curl --data @- example.com

which will read "hello world\n" from stdin and send it to example.com. We want to convert a command like echo "hello world" | curl --data @- example.com to Python. You can try pasting it on the website and it'll generate this:

import sys
import requests

headers = {
    'Content-Type': 'application/x-www-form-urlencoded',
}

data = sys.stdin.read().replace('\n', '').replace('\r', '').encode()

response = requests.post('http://example.com', headers=headers, data=data)

If you run curlconverter from the command line instead of the website it can convert it even better (because of the above mentioned code that reads stdin):

$ echo "hello world" | curlconverter --data @- example.com
import requests

headers = {
    'Content-Type': 'application/x-www-form-urlencoded',
}

data = 'hello world'

response = requests.post('http://example.com', headers=headers, data=data)

@rainydew
Copy link
Author

rainydew commented Jul 5, 2023

can we only enable this check when - arguement is placed

No, it's the opposite. This code only runs when there's no - argument to curlconverter. If you pass - that read happens here:

const input = fs.readFileSync(0, "utf8");

The idea of the code you're getting the error from is that you can have a curl command like this:

curl --data @- example.com

which means "read data from stdin and send it to example.com", so you could have a command like this

echo "hello world" | curl --data @- example.com

which will read "hello world\n" from stdin and send it to example.com. We want to convert a command like echo "hello world" | curl --data @- example.com to Python. You can try pasting it on the website and it'll generate this:

import sys
import requests

headers = {
    'Content-Type': 'application/x-www-form-urlencoded',
}

data = sys.stdin.read().replace('\n', '').replace('\r', '').encode()

response = requests.post('http://example.com', headers=headers, data=data)

If you run curlconverter from the command line instead of the website it can convert it even better (because of the above mentioned code that reads stdin):

$ echo "hello world" | curlconverter --data @- example.com
import requests

headers = {
    'Content-Type': 'application/x-www-form-urlencoded',
}

data = 'hello world'

response = requests.post('http://example.com', headers=headers, data=data)

thanks for this info.
i noticed it but this function doesn't work in many languages
e.g.

curl --data @- example.com

will generate PHP code like

<?php
require 'vendor/autoload.php';

use GuzzleHttp\Client;

$client = new Client();

$response = $client->post('http://example.com', [
    'headers' => [
        'Content-Type' => 'application/x-www-form-urlencoded'
    ],
    'body' => '@-'
]);

(regard as body)

or Java code like

import java.io.File;
import java.io.IOException;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;

OkHttpClient client = new OkHttpClient();

File file = new File("-");

Request request = new Request.Builder()
    .url("http://example.com")
    .post(file)
    .header("Content-Type", "application/x-www-form-urlencoded")
    .build();

try (Response response = client.newCall(request).execute()) {
    if (!response.isSuccessful()) throw new IOException("Unexpected code " + response);
    response.body().string();
}

(regard as filename)

or Go code

package main

import (
	"fmt"
	"io"
	"log"
	"net/http"
	"strings"
)

func main() {
	client := &http.Client{}
	var data = strings.NewReader(`@-`)
	req, err := http.NewRequest("POST", "http://example.com", data)
	if err != nil {
		log.Fatal(err)
	}
	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
	resp, err := client.Do(req)
	if err != nil {
		log.Fatal(err)
	}
	defer resp.Body.Close()
	bodyText, err := io.ReadAll(resp.Body)
	if err != nil {
		log.Fatal(err)
	}
	fmt.Printf("%s\n", bodyText)
}

so we can't implement this feature in real use till now, that's why we may at least not to enable std input for whole curl pattern without - or @- or anything like to read from stdin?

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

No branches or pull requests

2 participants