Red5d / pushbullet-bash

Bash interface to the PushBullet API
235 stars 41 forks source link

Script hangs while waiting for stdin #44

Open idefixx opened 9 years ago

idefixx commented 9 years ago

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 commented 9 years ago

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

ghost commented 8 years ago

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 commented 8 years ago

@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 commented 7 years ago

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 commented 7 years ago

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

https://github.com/fbartels/pushbullet-bash/commit/d35d967fb2f9b0912c8ab7c35eb7fb8acf2cf672

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

KronK0321 commented 7 years ago

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

Some warnings, though:

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

fbartels commented 7 years ago

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 commented 5 years ago

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")
    ...
  3. 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).