MSC4078: Transparent pusher creation #4078
Conversation
stefanceriu
changed the title
stefanceriu
force-pushed
the
stefan/pusher-creation
branch
from
a7883f9 to
a076ee6
Compare
stefanceriu
force-pushed
the
stefan/pusher-creation
branch
from
a076ee6 to
cc0dfa6
Compare
|
|
||
| ## Proposal | ||
|
|
||
| The most straight forward solution would be to directly expose the underlying errors on POST `/pushers`. We cannot know beforehand what the final service used is so we shouldn't attempt to wrap errors and just rely on HTTP semantics. |
There was a problem hiding this comment.
I think I'm not quite following what's happening; is it the following:
- The client configures a pusher.
- The homeserver happily uses that pusher for a bit.
- For some reason, the push service starts erroring.
- The error gets receive by the push gateway which passes it on.
- The homeserver receives the error and deletes the pusher.
- The user receives no more pushes.
- On next app launch (?) we go back to 1.
Is that accurate or is something else happening?
I'm unsure why POST /pushers/set is the proper place to return an error? How would the homeserver even track that?
There was a problem hiding this comment.
On next app launch (?) we go back to 1.
yes, but this time round the the token will be reported as invalid by the APNs and the error will be forwarded back from synapse to the client. The client can then requests a new token from Apple and start again from 1.
I'm unsure why POST /pushers/set is the proper place to return an error? How would the homeserver even track that?
talked about this offline: we believe the best solution is to check that the pusher config is actually valid as soon the creation request comes in as opposed to deferring it until trying to send the first push and then failing silently.
stefanceriu
force-pushed
the
stefan/pusher-creation
branch
from
0211bf8 to
2d3d06a
Compare
|
|
||
| ## Proposal | ||
|
|
||
| The most straight forward solution would be to directly expose the underlying errors on POST `/pushers`. We cannot know |
There was a problem hiding this comment.
I am not sure if the actual error of the backend is actually useful for the client. Not only is it not helpful for the enduser if it seems them but also for the app it might be a variety of used backends so error handling probably ends up being a catch all regardless. I think a generic "error happened" is enough here unless I miss some usecase here.
There was a problem hiding this comment.
I'm not sure either to be honest. For APNs for example there's quite a big difference between a recoverable 410 and a completely unrecoverable 403, we wouldn't want the app to keep trying over and over again for no reason..
There was a problem hiding this comment.
I guess replicating the temp fail vs permanent fail could make sense yeah. That allows for a banner like "notification is currently unavailable" or a automatic preregister attempt.
There was a problem hiding this comment.
There's also quite a few layers to pass the error back through, e.g. push server -> push gateway -> homeserver -> client. This will need care to not mangle it.
stefanceriu
force-pushed
the
stefan/pusher-creation
branch
from
2d3d06a to
6b9153c
Compare
turt2live
added
push
proposal
|
The SCT has been asked to review this before implementation begins, as implementation is non-trivial. Thoughts welcome. |
Registering pushers against push notification services should forward back failures.
Solves element-hq/element-x-ios#991 (comment)
Rendered