julienXX / terminal-notifier

Send User Notifications on macOS from the command-line.
Other
6.37k stars 330 forks source link

terminal-notification exec is blocking #223

Closed uudashr closed 6 years ago

uudashr commented 7 years ago

Please check everything that applies to your issue:

Mac Version: macSierra 10.12.5 terminal-notifier version: 1.7.0 Installation method: homebrew Step-by-step reproduction:

$ terminal-notifier -message "Lorem ipsum dolor sit amet" -title "Lorem" -activate "com.googlecode.iterm2"

It blocks instead exiting the command

dkristian commented 7 years ago

I can reproduce this too on macOS 10.11.6 (15G1510) after updating to 1.8.0 (via MacPorts) when I pass the optional -open argument. The program only exits straight away if I click the notification before it disappears.

I suspect the fact that it's not exiting cleanly is the reason why I started receiving mail from cron as mentioned in https://github.com/julienXX/terminal-notifier/issues/218#issuecomment-303924021.

uudashr commented 7 years ago

Same with me... click the notif then continue. Maybe it blocks because wait for click input, that is why it not exiting

chew-z commented 7 years ago

+1

chew-z commented 7 years ago

And when I click on notification I get

2017-05-27 11:34:48.920 terminal-notifier[52134:803653] @CONTENTCLICKED
majorsl commented 7 years ago

Noted this too with 1.8. Even though it isn't supposed to be necessary, I added a -timeout 10 as a temporary work around until the bug is squashed.

rr0ss0rr commented 7 years ago

I thought the issue was terminal-notifier 1.8 changed the notification type in error to Alerts. I changed it back to Banners in Notification Center.

rikai commented 7 years ago

I can confirm this with -activate as well. Updated to 1.8 in brew and it stalled out the scripts i was using terminal-notifier as a part of completely, as it blocks further execution. 🚨

shalecraig commented 7 years ago

I've been able to isolate this test case down to this statement:

When passed the -activate flag, the application hangs until someone clicks the item.

terminal-notifier -message "hi" -activate "whatever"

I suspect that a large number of people are running into this because BGNotify, as seen in https://github.com/t413/zsh-background-notify (and notably, oh-my-zsh), passes the -activate flag through.

shalecraig commented 7 years ago

Some more digging leads me to strongly suspect that one of defaults[@"execute"] || defaults[@"open"] || options[@"bundleID"] evaluates to true, causing options[@"waitForResponse"] to be set to @YES. (source here)

bundleId is sourced from activate, which is the param that seems to trigger this behaviour in our bug repro.

Git blame points me to the @julienXX 's recent change to make macos 10.8 / 10.9 no longer supported: https://github.com/julienXX/terminal-notifier/commit/1d40bcf782e0037115233f32bbf38ad121438661#diff-4f90fe8a7b85b78017139b266d8ad8fbR205

It's tempting to think that "If you're passing activate, you'll want the window to stick around", but I don't think you'd want it to block execution unless you also passed in dropdownLabel (or something else, maybe...)

It doesn't look like this clause was that broad on purpose, I've opened pull-request https://github.com/julienXX/terminal-notifier/pull/225 to make it more specific.

chew-z commented 7 years ago

Can't that be fixed with some quick patch? I had to switch off notifications for most of my AppleScript services because they hang otherwise forever waiting to hear back from terminal-notifier. Quite annoying.

julienXX commented 7 years ago

@chew-z no, quick patch is always the wrong way to go. I'm working on it, in the meantime you can revert to 1.7.2.

volans- commented 6 years ago

Is there any news/progress towards the solution of this?

gms8994 commented 6 years ago

This happens to me as well without using -activate, but instead using -timeout:

Mac Version: macSierra 10.12.5 terminal-notifier version: 1.8.0 Installation method: rubygems

echo "HI" | terminal-notifier -title "TEST" -timeout 10

I get a response of 2017-10-26 10:36:12.273 terminal-notifier[33324:20852242] @TIMEOUT.

julienXX commented 6 years ago

Version 2.0.0 as been released with alerter features removed https://github.com/julienXX/terminal-notifier/releases/tag/2.0.0 so this issue is not relevant anymore as there is no more waiting for user action. Closing for now, feel free to re-open.