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

Script hangs while waiting for stdin #44

Open
idefixx opened this issue Aug 12, 2015 · 8 comments
Open

Script hangs while waiting for stdin #44

idefixx opened this issue Aug 12, 2015 · 8 comments

Comments

@idefixx
Copy link

idefixx commented Aug 12, 2015

This is so we can pipe notes to the script, right? It causes issues when run from other scripts though.
Used pushbullet-bash in Deluge for notifications and as an upstart job in both cases it seemed to think a terminal was connected and hung at 'cat'. Not the best solution I guess.

        if [ ! -t 0 ]; then
            # we have something on stdin
            body=$(cat)
@Red5d
Copy link
Owner

Red5d commented Aug 13, 2015

Good catch, thanks. I'll see what I can do to fix that.

@ghost
Copy link

ghost commented May 24, 2016

Is it fixed? I have a problem with Deluge execute script
[WARNING ] 18:22:17 core:132 [execute] command '~/torrentDownloadCompleted.sh' failed with exit code 1 inside I have:

`#!/bin/zsh

torrentId=$1
torrentName=$2
torrentFolder=$3

~/pushbullet-bash/pushbullet push all note "Deluge" "$torrentName" &> /dev/null
`

@fbartels
Copy link
Collaborator

@Haxy89 I am not really familiar with zsh, but the exit 1 can originate from the fact that you should call pushbullet-bash like ./pushbullet and not ~/pushbullet.

As @idefixx said pushbullet-bash will hang if the script does not find something in the pipe and not fail. But honestly I think the issue is more with deluge as with pushbullet-bash as test ! -t 0 is a valid solution.

@KronK0321
Copy link
Contributor

I'd like to add that I experienced the same issue calling pushbullet from a bash script.

#!/bin/bash
{ 
/usr/bin/pushbullet push $channelname note "${TR_TORRENT_NAME}" "$(date)\nPosttorrent success\n${TR_TORRENT_NAME}\n${TR_TORRENT_DIR}" | tee -a $LOG_MAIN
} &

My script wouldn't exit because pushbullet got hung up invoking cat even though there was nothing passed to it via stdin.
I've commented out the entire IF block as a hack-ish workaround. I've looked for other solutions to no avail.

#  if [ ! -t 0 ]; then                                                                                             #     # we have something on stdin
#     body=$(cat)
#     # remove unprintable characters, or pushbullet API fails
#     body=$(echo "$body"|tr -dc '[:print:]\n'|tr '"' "'")
#  fi

@fbartels
Copy link
Collaborator

I played a bit with the following approach some months ago, but never pushed it upstream:

fbartels@d35d967

Can you see if that does the trick for you as well? Then I can create a pull request for this.

@KronK0321
Copy link
Contributor

KronK0321 commented Feb 19, 2017

In my rudimentary testing, your patch does allow my script to execute correctly.

Some warnings, though:

  • I experienced the pushbullet issue over 9 months ago, commented out the offending lines and continued developing my script. It may have changed since then.
  • I was unable to make a very basic script to call pushbullet and reproduce the problem when the line was if [ ! -t 0 ]

So I'm certainly not a reliable test as to whether this definitely fixes the issue, sorry!

@fbartels
Copy link
Collaborator

fbartels commented Mar 7, 2017

Yes, thats my experience as well. If you turn to places like stackoverflow, you get a lot if answers suggesting to implement it like it is in the current pushbullet-bash version. The change I tested with has the benefit of being able to timeout after a while. But in general to me it seems more of a problem of the calling script, than pushbullet-bash itself.

But thats just my two cents.

@Osndok
Copy link

Osndok commented Nov 27, 2018

IMO, the proper way to fix this issue would be to:

  1. introduce a '--body' flag that takes a file path argument (in this example being assigned to the BODY_FILE variable)
  2. Replace the offending lines with something like:
if [ -n "$BODY_FILE" ]
then
    body=$(cat "$BODY_FILE")
...
  1. Document that if people want to pass a message body via stdin, they should use --body - (where the minus sign is customary to mean "stdin", and cat already accepts).

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

No branches or pull requests

5 participants