6
61
Fork
You've already forked android
5

Toast displayed on every registration is distracting #30

Closed
opened 2025年04月26日 13:49:43 +02:00 by nikclayton · 17 comments

UnifiedPush recommends that apps re-register on startup.

From time to time, like every time you starts your application, you should register your application in case the user have uninstalled the previous distributor.
-- https://unifiedpush.org/kdoc/connector/

and

  1. From time to time, like every time the application starts, C register to D to avoid inconsistent state
    -- https://unifiedpush.org/developers/spec/android/

Because Sunup shows a Toast when this happens, every time an app starts the user sees an unhelpful message (looks like "$appname registered.").

It's unhelpful because it doesn't tell the user anything actionable. Successful registrations should be silent, and any errors that occur during registration should be passed back to the client application to display them to the user in a way that makes sense for that app.

Please remove this, thanks.

https://scholar.social/@gedankenstuecke/114401133179804192 is an example user complaint about this behaviour.

UnifiedPush recommends that apps re-register on startup. > From time to time, like every time you starts your application, you should register your application in case the user have uninstalled the previous distributor. > -- https://unifiedpush.org/kdoc/connector/ and > 3. From time to time, like every time the application starts, C register to D to avoid inconsistent state > -- https://unifiedpush.org/developers/spec/android/ Because Sunup shows a Toast when this happens, every time an app starts the user sees an unhelpful message (looks like "$appname registered."). <img src="/attachments/2bc7813e-903c-4a0a-a5be-83a10a61d890" width="45%"> It's unhelpful because it doesn't tell the user anything actionable. Successful registrations should be silent, and any errors that occur during registration should be passed back to the client application to display them to the user in a way that makes sense for that app. Please remove this, thanks. https://scholar.social/@gedankenstuecke/114401133179804192 is an example user complaint about this behaviour.

User in question here, just to say that I'd be happy to help with any user testing and giving more feedback if useful 🙂

User in question here, just to say that I'd be happy to help with any user testing and giving more feedback if useful 🙂
Owner
Copy link

What version of Sunup do you use ? This should not appear during a registration update

What version of Sunup do you use ? This should not appear during a registration update

No problem, I installed Sunup via obtainium, see Details in the screenshot!

No problem, I installed Sunup via obtainium, see Details in the screenshot!
Owner
Copy link

And everytime you restart pachli, it shows this toast ?

And everytime you restart pachli, it shows this toast ?
Owner
Copy link

By the way, do you know how to get an application logs ? Or do you mind to test ? Having sunup logs would help

By the way, do you know how to get an application logs ? Or do you mind to test ? Having sunup logs would help

Yep, everytime I open Pachli after it was closed (either manually or due to running out of memory)

Yep, everytime I open Pachli after it was closed (either manually or due to running out of memory)

I don't know but I also don't mind getting them!

I don't know but I also don't mind getting them!

Got it I think, let me know if that's helpful!

type: logcat
osVersion: google/tegu/tegu:15/BD4A.250405.003/2025041700:user/release-keys
package: org.unifiedpush.distributor.sunup:10, targetSdk 35
buffers: main,system,crash,events,kernel
level: verbose
04-28 11:15:34.910 6359 6359 D WakeLock: Acquiring WakeLock.
04-28 11:15:34.914 6359 6359 I UP_Distrib: UNREGISTER
04-28 11:15:34.934 6359 6359 D e : Sending {"messageType":"unregister","channelID":"[REMOVED-1]"}
04-28 11:15:34.940 6359 6359 D UP_Distrib: Refreshing nRegistrations to 0
04-28 11:15:34.943 6359 6359 D UP_Distrib: Starting [61a99aca-5fd9-4cf4-ac73-5a4c167b4a64] is 0 ms
04-28 11:15:34.957 6359 6359 D UP_Distrib: Unregistration is finished
04-28 11:15:34.957 6359 6359 D WakeLock: Releasing WakeLock.
04-28 11:15:34.960 6359 6359 D WakeLock: Acquiring WakeLock.
04-28 11:15:34.960 6359 6359 I UP_Distrib: REGISTER
04-28 11:15:34.962 6359 6359 D UP_Distrib: Registering app.pachli.current. Package name retrieved with legacy String extra
04-28 11:15:34.966 6359 6359 D UP_Distrib: New registration for app.pachli.current
04-28 11:15:34.969 6359 6359 D e : Sending {"messageType":"register","channelID":"[REMOVED-2]"}
04-28 11:15:34.973 6359 6359 D UP_Distrib: Refreshing nRegistrations to 1
04-28 11:15:34.973 6359 6359 D UP_Distrib: Starting [35fb636a-3453-4f41-9081-0cd47d724c22] is 0 ms
04-28 11:15:34.982 6359 6359 D WakeLock: Releasing WakeLock.
04-28 11:15:35.175 6359 9704 D f : New message: T
04-28 11:15:35.178 6359 9704 D f : New message: P
04-28 11:15:35.179 6359 9704 D f : New endpoint: https://updates.push.services.mozilla.com/wpush[REMOVED]
04-28 11:15:35.215 6359 10866 D RestartWorker: Working [35fb636a-3453-4f41-9081-0cd47d724c22]
04-28 11:15:35.215 6359 10866 D RestartWorker: Running without failure
04-28 11:15:35.215 6359 10866 D e : Sending {}
04-28 11:15:35.490 6359 9704 D f : New message: M
04-28 11:15:35.490 6359 9704 D f : Received Pong
04-28 11:16:38.287 6359 6432 D RestartWorker: Working [7fbc4f3a-ed65-4b67-8159-a71839399e2d]
04-28 11:16:38.287 6359 6432 D RestartWorker: Running without failure
04-28 11:16:38.287 6359 6432 D e : Sending {}
04-28 11:16:38.508 6359 9704 D f : New message: M
04-28 11:16:38.508 6359 9704 D f : Received Pong
04-28 11:18:23.988 6359 6359 D WakeLock: Acquiring WakeLock.
04-28 11:18:23.990 6359 6359 I UP_Distrib: UNREGISTER
04-28 11:18:23.993 6359 6359 D e : Sending {"messageType":"unregister","channelID":"[REMOVED-2]"}
04-28 11:18:23.995 6359 6359 D UP_Distrib: Refreshing nRegistrations to 0
04-28 11:18:23.996 6359 6359 D UP_Distrib: Starting [4fe96bcd-86bb-4244-a643-4932bb116f0b] is 0 ms
04-28 11:18:24.001 6359 6359 D UP_Distrib: Unregistration is finished
04-28 11:18:24.001 6359 6359 D WakeLock: Releasing WakeLock.
04-28 11:18:24.006 6359 6359 D WakeLock: Acquiring WakeLock.
04-28 11:18:24.007 6359 6359 I UP_Distrib: REGISTER
04-28 11:18:24.007 6359 6359 D UP_Distrib: Registering app.pachli.current. Package name retrieved with legacy String extra
04-28 11:18:24.009 6359 6359 D UP_Distrib: New registration for app.pachli.current
04-28 11:18:24.010 6359 6359 D e : Sending {"messageType":"register","channelID":"[REMOVED-3]"}
04-28 11:18:24.011 6359 6359 D UP_Distrib: Refreshing nRegistrations to 1
04-28 11:18:24.011 6359 6359 D UP_Distrib: Starting [eca5db8b-1931-4dba-96f2-8590a1a9fa50] is 0 ms
04-28 11:18:24.020 6359 6359 D WakeLock: Releasing WakeLock.
04-28 11:18:24.231 6359 6456 D RestartWorker: Working [eca5db8b-1931-4dba-96f2-8590a1a9fa50]
04-28 11:18:24.231 6359 6456 D RestartWorker: Running without failure
04-28 11:18:24.232 6359 6456 D e : Sending {}
04-28 11:18:24.241 6359 9704 D f : New message: T
04-28 11:18:24.242 6359 9704 D f : New message: P
04-28 11:18:24.242 6359 9704 D f : New endpoint: https://updates.push.services.mozilla.com/wpush[REMOVED]
04-28 11:18:24.477 6359 9704 D f : New message: M
04-28 11:18:24.477 6359 9704 D f : Received Pong
Got it I think, let me know if that's helpful! ``` type: logcat osVersion: google/tegu/tegu:15/BD4A.250405.003/2025041700:user/release-keys package: org.unifiedpush.distributor.sunup:10, targetSdk 35 buffers: main,system,crash,events,kernel level: verbose 04-28 11:15:34.910 6359 6359 D WakeLock: Acquiring WakeLock. 04-28 11:15:34.914 6359 6359 I UP_Distrib: UNREGISTER 04-28 11:15:34.934 6359 6359 D e : Sending {"messageType":"unregister","channelID":"[REMOVED-1]"} 04-28 11:15:34.940 6359 6359 D UP_Distrib: Refreshing nRegistrations to 0 04-28 11:15:34.943 6359 6359 D UP_Distrib: Starting [61a99aca-5fd9-4cf4-ac73-5a4c167b4a64] is 0 ms 04-28 11:15:34.957 6359 6359 D UP_Distrib: Unregistration is finished 04-28 11:15:34.957 6359 6359 D WakeLock: Releasing WakeLock. 04-28 11:15:34.960 6359 6359 D WakeLock: Acquiring WakeLock. 04-28 11:15:34.960 6359 6359 I UP_Distrib: REGISTER 04-28 11:15:34.962 6359 6359 D UP_Distrib: Registering app.pachli.current. Package name retrieved with legacy String extra 04-28 11:15:34.966 6359 6359 D UP_Distrib: New registration for app.pachli.current 04-28 11:15:34.969 6359 6359 D e : Sending {"messageType":"register","channelID":"[REMOVED-2]"} 04-28 11:15:34.973 6359 6359 D UP_Distrib: Refreshing nRegistrations to 1 04-28 11:15:34.973 6359 6359 D UP_Distrib: Starting [35fb636a-3453-4f41-9081-0cd47d724c22] is 0 ms 04-28 11:15:34.982 6359 6359 D WakeLock: Releasing WakeLock. 04-28 11:15:35.175 6359 9704 D f : New message: T 04-28 11:15:35.178 6359 9704 D f : New message: P 04-28 11:15:35.179 6359 9704 D f : New endpoint: https://updates.push.services.mozilla.com/wpush[REMOVED] 04-28 11:15:35.215 6359 10866 D RestartWorker: Working [35fb636a-3453-4f41-9081-0cd47d724c22] 04-28 11:15:35.215 6359 10866 D RestartWorker: Running without failure 04-28 11:15:35.215 6359 10866 D e : Sending {} 04-28 11:15:35.490 6359 9704 D f : New message: M 04-28 11:15:35.490 6359 9704 D f : Received Pong 04-28 11:16:38.287 6359 6432 D RestartWorker: Working [7fbc4f3a-ed65-4b67-8159-a71839399e2d] 04-28 11:16:38.287 6359 6432 D RestartWorker: Running without failure 04-28 11:16:38.287 6359 6432 D e : Sending {} 04-28 11:16:38.508 6359 9704 D f : New message: M 04-28 11:16:38.508 6359 9704 D f : Received Pong 04-28 11:18:23.988 6359 6359 D WakeLock: Acquiring WakeLock. 04-28 11:18:23.990 6359 6359 I UP_Distrib: UNREGISTER 04-28 11:18:23.993 6359 6359 D e : Sending {"messageType":"unregister","channelID":"[REMOVED-2]"} 04-28 11:18:23.995 6359 6359 D UP_Distrib: Refreshing nRegistrations to 0 04-28 11:18:23.996 6359 6359 D UP_Distrib: Starting [4fe96bcd-86bb-4244-a643-4932bb116f0b] is 0 ms 04-28 11:18:24.001 6359 6359 D UP_Distrib: Unregistration is finished 04-28 11:18:24.001 6359 6359 D WakeLock: Releasing WakeLock. 04-28 11:18:24.006 6359 6359 D WakeLock: Acquiring WakeLock. 04-28 11:18:24.007 6359 6359 I UP_Distrib: REGISTER 04-28 11:18:24.007 6359 6359 D UP_Distrib: Registering app.pachli.current. Package name retrieved with legacy String extra 04-28 11:18:24.009 6359 6359 D UP_Distrib: New registration for app.pachli.current 04-28 11:18:24.010 6359 6359 D e : Sending {"messageType":"register","channelID":"[REMOVED-3]"} 04-28 11:18:24.011 6359 6359 D UP_Distrib: Refreshing nRegistrations to 1 04-28 11:18:24.011 6359 6359 D UP_Distrib: Starting [eca5db8b-1931-4dba-96f2-8590a1a9fa50] is 0 ms 04-28 11:18:24.020 6359 6359 D WakeLock: Releasing WakeLock. 04-28 11:18:24.231 6359 6456 D RestartWorker: Working [eca5db8b-1931-4dba-96f2-8590a1a9fa50] 04-28 11:18:24.231 6359 6456 D RestartWorker: Running without failure 04-28 11:18:24.232 6359 6456 D e : Sending {} 04-28 11:18:24.241 6359 9704 D f : New message: T 04-28 11:18:24.242 6359 9704 D f : New message: P 04-28 11:18:24.242 6359 9704 D f : New endpoint: https://updates.push.services.mozilla.com/wpush[REMOVED] 04-28 11:18:24.477 6359 9704 D f : New message: M 04-28 11:18:24.477 6359 9704 D f : Received Pong ```
Owner
Copy link

@nikclayton : When the application starts, you are calling EnableAllNotificationsUseCase that unregister the instance before registering it again, this can be seen in @gedankenstuecke logs

When you start the application, it is indeed recommended to call register, but you should not call unregister first. If you call register directly, the distributor will send the existing endpoint if it isn't updated. Do you think the documentation (kdoc) should be updated ?

@nikclayton : When the application starts, you are calling [EnableAllNotificationsUseCase](https://github.com/pachli/pachli-android/blob/d1235a376a4d41bad737b941774c52930c300b86/app/src/main/java/app/pachli/components/notifications/domain/EnableAllNotificationsUseCase.kt#L33) that unregister the instance before registering it again, this can be seen in @gedankenstuecke logs When you start the application, it is indeed recommended to call _register_, but you should not call _unregister_ first. If you call _register_ directly, the distributor will send the existing endpoint if it isn't updated. Do you think the documentation (kdoc) should be updated ?

IIRC, that was to work around a problem last year where the UnifiedPush client library could get in to a state where it would silently fail (v2.4.0 at the time).

But that's slightly orthogonal to this issue I think, as there's still no need for the app to show a toast in this situation. The client (Pachli in this case) can show an error on failure, but success should be silent.

IIRC, that was to work around a problem last year where the UnifiedPush client library could get in to a state where it would silently fail (v2.4.0 at the time). But that's slightly orthogonal to this issue I think, as there's still no need for the app to show a toast in this situation. The client (Pachli in this case) can show an error on failure, but success should be silent.
Owner
Copy link

I don't know if it is orthogonal, without the unregister request during startup, we wouldn't have that issue. And without the toast we would have missed the unwanted unregister isn't it ?

The reason there is that toast for new registrations is that UnifiedPush can be enabled in different places in applications, and it follows a user request (a setting toggle on for instance). We have seen users trying to "create channels" on the distributor manually, or looking around to enable UnifiedPush while it is already enabled. This toast is here to inform that the connection is done, and works, so they don't have anything more to do.

Maybe we can introduce a setting to disable toasts, but I'd like to avoid unnecessary settings

I don't know if it is orthogonal, without the unregister request during startup, we wouldn't have that issue. And without the toast we would have missed the unwanted unregister isn't it ? The reason there is that toast for _new_ registrations is that UnifiedPush can be enabled in different places in applications, and it follows a user request (a setting toggle on for instance). We have seen users trying to "create channels" on the distributor manually, or looking around to enable UnifiedPush while it is already enabled. This toast is here to inform that the connection is done, and works, so they don't have anything more to do. Maybe we can introduce a setting to disable toasts, but I'd like to avoid unnecessary settings

Just to understand: Is this something that Pachli could be fixing by not deregistering? If so, should I open an issue in the Pachli repo @nikclayton?

Just to understand: Is this something that Pachli could be fixing by not deregistering? If so, should I open an issue in the Pachli repo @nikclayton?

I'm still not convinced that calling unregister before calling register is a bug in Pachli. As I say, this was done to work around a bug in the UnifiedPush libraries -- if not done the client could silently fail to register, leaving users (and me) very confused as to why notifications were not being delivered.

So I'm not sure that describing it as an "unwanted unregister" is accurate -- it is very much wanted, in order to ensure that registration does actually happen.

I think it's up to the client to clearly show the state of registration, to avoid the user confusion issue that @s1m mentioned.

By analogy -- suppose I was using a third party library in a terminal app, and that third party library started sending debugging output to STDOUT, interfering with the output my app was producing. That would definitely be a bug -- the only thing producing user-visible output should be my code. I think that's the same case here -- it would be reasonable for the distributor to log the registration, but I think that interfering[1] with the client's UI is not appropriate.

[1] I appreciate that this is interference in a practical sense, not a strictly technical sense.

I'm still not convinced that calling *unregister* before calling *register* is a bug in Pachli. As I say, this was done to work around a bug in the UnifiedPush libraries -- if not done the client could silently fail to register, leaving users (and me) very confused as to why notifications were not being delivered. So I'm not sure that describing it as an "unwanted unregister" is accurate -- it is very much wanted, in order to ensure that registration does actually happen. I think it's up to the client to clearly show the state of registration, to avoid the user confusion issue that @s1m mentioned. By analogy -- suppose I was using a third party library in a terminal app, and that third party library started sending debugging output to STDOUT, interfering with the output my app was producing. That would definitely be a bug -- the only thing producing user-visible output should be my code. I think that's the same case here -- it would be reasonable for the distributor to **log** the registration, but I think that interfering[1] with the client's UI is not appropriate. --- [1] I appreciate that this is interference in a practical sense, not a strictly technical sense.
Owner
Copy link

The spec are designed to get registration requests from registrations they already know, explicitly for this use case:

https://unifiedpush.org/developers/spec/android/#orgunifiedpushandroidconnectornew_endpoint

The distributor MUST send this action to the registered application in the following cases: [...] the end user application requested a registration (org.unifiedpush.android.distributor.REGISTER) with a token it is already using

And 3 different state for new endpoints are defined.

Sunup (,NextPush and gCompat) shows a toast for new registration requests (with a new token, not the updated ones). Pachli always send new registration requests. So yes, pachli should not unregister then register. We can improve the doc if you need, but this has been designed that way

Then we can debate about the toast. It is a kind of interruption from another app. But as I said, it helps user to understand when things are correctly setup. I think this is a compromise that's worth it

The spec are designed to get registration requests from registrations they already know, explicitly for this use case: https://unifiedpush.org/developers/spec/android/#orgunifiedpushandroidconnectornew_endpoint > The distributor MUST send this action to the registered application in the following cases: [...] the end user application requested a registration (org.unifiedpush.android.distributor.REGISTER) with a token it is already using And 3 different state for new endpoints are defined. Sunup (,NextPush and gCompat) shows a toast for new registration requests (with a new token, not the updated ones). Pachli always send _new_ registration requests. So yes, pachli should not unregister then register. We can improve the doc if you need, but this has been designed that way Then we can debate about the toast. It is a kind of interruption from another app. But as I said, it helps user to understand when things are correctly setup. I think this is a compromise that's worth it
Owner
Copy link

I think I'll add a setting to toggle on/off the toasts for the next release

I think I'll add a setting to toggle on/off the toasts for the next release
Owner
Copy link

The toasts are now behind a toggle: 7c2bc7eac7

The toasts are now behind a toggle: https://codeberg.org/Sunup/android/commit/7c2bc7eac7cfcc38e4451ec9a3b274eace2d8432

Thanks!

Thanks!
Sign in to join this conversation.
No Branch/Tag specified
main
1.2.3
1.2.3-rc1
1.2.2
1.2.1
1.2.0
1.1.0
1.0.4
1.0.3
1.0.2
1.0.1
1.0.0
0.2.2
0.2.1
0.2.0
0.1.0
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Sunup/android#30
Reference in a new issue
Sunup/android
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?