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

dunstify hangs indefinitely when using Action and get replaced #672

Closed
Jonathas-Conceicao opened this issue Nov 12, 2019 · 7 comments
Closed

Comments

@Jonathas-Conceicao
Copy link

If dunstify is called with Action, and is then replaced by some new notification, the first call will then be left hanging indefinitely. e.g:

Form shell 1:
dunstify -r 313 "Testing" -A 'tested,default'
After that, from a new shell:
dunstify -r 313 "New Test"

The call on shell 1 will never finish. However If a -C 313 is used before sending the second notification, the first call to dunstify exits with a 3, as expected.

Is this the desired behaviour or a bug? I would expect a replace call to also close the notification with the conflicting id.

Installation info

  • Version: Dunst - A customizable and lightweight notification-daemon 1.4.1 (2019-07-03)
  • Install type: package
  • Distro and version: Arch Linux
@tsipinakis
Copy link
Member

It's sadly an unfortunate side effect of how replacing is being done.

It might seem logical that if a notification is replaced we should notify the first sender that it was closed.

However, as I read this feature of the notification spec it's meant to be used within the same program to update an existing notification, which would lead to a very confusing callback if we send a NotificationClosed message when it was just replaced.

So it's indeed the logical thing to do for dunstify, but for other (more common) use-cases we might end up confusing or straight up breaking other programs.

@progandy
Copy link
Contributor

progandy commented Nov 14, 2019

doesn't dbus identify the sender? Maybe store that in the notification structure and only send the close event when it differs?

Edit: That is already stored in n->dbus_client

@tsipinakis
Copy link
Member

tsipinakis commented Nov 15, 2019

doesn't dbus identify the sender?

Technically it's possible, but I would classify it into the 'ugly hack' category and be very hesitant to merge that into the main tree as we're stepping on the lines on what's properly defined and not so it could lead to some very confusing behaviour down the road.


@Jonathas-Conceicao This specific behaviour aside, what's the use-case you're actually trying to accomplish here, are you aware of stack_tag (some documentation available in the manpage - but it's pretty sparse)?

e.g. Notifications with the same tag ("test" in this example) are replaced without having to care for ids.
This works correctly:

$ ./dunstify -h string:x-dunst-stack-tag:test Test -A 'tested,default'
$ ./dunstify -h string:x-dunst-stack-tag:test Testing

For higher level uses it's best to stick to using that rather than micro-managing IDs as that has many hidden pitfalls.
Granted, this will only work with a recent-ish version of dunst and not any other notification daemon.

@progandy
Copy link
Contributor

we're stepping on the lines on what's properly defined and not so it could lead to some very confusing behaviour down the road.

True. I think even the ability to replace or close notifications that were created by a different client is on that line.

@tsipinakis
Copy link
Member

Indeed this would fit right into dunsts id handling which is why I didn't dismiss that idea.

Dunst has always handled ids weirdly deviating a bit from the spec. i.e in the spec it's not allowed to use replaces_id for anything other than replacing an existing, active notification.
In contrast, dunst will allow clients to arbitrarily choose the id they want allocated by setting replaces_id, which sometimes works to the systems detriment as it can cause clashes when the id counter and forced replace ids occur.


For this reason we implemented stack_tag(see more discussion in #552 and linked issues), to get away from this weird handling of ids.

So, aside from all the uncertainties, I would be all for adding this if we weren't trying to move away from using -r in this way.
In my mind -r is now reserved for debugging and for very complex notification senders and at that point, it's probably better to use python+libnotify that handles all this stuff behind the scenes.

@Jonathas-Conceicao
Copy link
Author

This specific behaviour aside, what's the use-case you're actually trying to accomplish here

I have a script that send notifications with music lyrics stanza by stanza and I use Actions to pass through them. I eventually noticed some calls to dunstify were hanging indefinitely when a new music on spotify started and the notification for the new song replaced the old one.

are you aware of stack_tag?

I have had a quick look on it when I was writing my script, but as replacing seamed easier to use and I though it would do the trick I went for it. I have tested with it now and it does seams to sove my problems.

@Jonathas-Conceicao
Copy link
Author

Jonathas-Conceicao commented Nov 25, 2019

I'll be closing this since replace doesn't send a close to the old notification by design and the desired behaviour can be accomplished by using stack_tag.

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

3 participants