-
Notifications
You must be signed in to change notification settings - Fork 45
Add push notifications guide #2983
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
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1b7f8ce to
d429c0b
Compare
d429c0b to
828e537
Compare
| ```kotlin | ||
| // Android | ||
| val ably = getAblyRealtime() | ||
| ably.setAndroidContext(context) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it complex to show here what context is?
| <Code> | ||
| ```javascript | ||
| // Update device registration | ||
| await client.push.admin.deviceRegistrations.save({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an API to delete registrations as well?
| } | ||
| })); | ||
|
|
||
| // Send up to 10,000 notifications in one request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we charge per REST request or per notification?
|
|
||
| Push notification failures are inevitable: devices go offline, users uninstall apps, tokens expire. Monitoring and handling errors properly is critical. | ||
|
|
||
| Ably publishes push errors to a dedicated meta channel `[meta]log:push`. Monitor this channel to track push notification delivery errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a channel reserved name? Does this channel exist for all the accounts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we bill for the messages sent to this meta channel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes to all three questions.
AndyTWF
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, a few general thoughts:
- Where we have codeblocks, we should have them in all the languages that are relevant. We should also include other Pub/Sub languages (e.g. Go, Ruby) if the SDKs support it.
- Can we link the architecture docs somewhere to hammer home the scale aspect? We should also continue to make reference to our four pillars, SLA and uptime to make it clear why Ably is the right choice at scale.
- We should discuss tradeoffs in various decisions in terms of pricing, where that's applicable.
- Can we add a mockup of a PN (happy for this to be designed) to illustrate?
- From about half way through, things start to turn into an extended docs page - can we try to refocus them more on where Ably differentiates and excels at scale?
| ```javascript | ||
| // Subscribe to all channels, and push-subscribe for "news:" channels; suitable for a client: | ||
| { | ||
| '*': ['subscribe'], // Can subscribe to realtime messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is lacking context - where do these things get passed in? We should probably call out where this would be set up.
Possibly worth making this a JSON block and not a code block, otherwise we need to add additional languages.
| Follow these practices: | ||
|
|
||
| * Never give clients `push-admin` capability. They could manage registrations and subscriptions for push notifications for any device. | ||
| * Do not give `publish` capability to clients on channels used for push notifications. This could allow any clients to publish push notifications to that channel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a hard no, or just a recommendation? Is there a case where someone might want this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My take is that I can't see a use case to be honest, it's all risky to allow it. So hard no, unless you know what you're doing, in which case you'll ignore this sentence anyway (and it includes why not so I'd leave it in). More trouble than worth?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to make it stronger
| * Do not give `publish` capability to clients on channels used for push notifications. This could allow any clients to publish push notifications to that channel. | |
| * Do not give `publish` capability to clients on channels used for push notifications. This will allow any clients to publish push notifications to that channel. |
| When users disable notifications in your app or log out, remove the device registration from Ably: | ||
|
|
||
| <Code> | ||
| ```javascript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other languages?
| * Any other business logic. | ||
|
|
||
| <Code> | ||
| ```javascript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't put this as a code block - as otherwise we have to duplicate across each (and it isn't code).
| }); | ||
| ``` | ||
|
|
||
| ```java |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is server-side, lets also add:
- Ruby
- Go
- PHP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and the publish example, it makes sense.
Go doesn't seem to have push admin yet so it'll look like rest.Request() but that should be fine. Maybe a curl example will be good too to show the API. Ruby seems to have it. Will check PHP. Probably python too, it looks like we might have push admin for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
|
||
| Ably publishes delivery feedback to the `[meta]log:push` channel in real-time. This gives you immediate visibility into failures - invalid tokens, expired registrations, platform outages - so you can maintain a clean device registry and debug issues quickly. Ably also handles automatic retries when APNs or FCM experience transient issues, so you don't need to build retry logic yourself. | ||
|
|
||
| ## Production-ready checklist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before the production ready checklist - I think we should try to come back to one final "why Ably" callout, even if it's just a few sentences. It gives us a chance to call out the key USPs, scale etc - one final emphasis on where Ably wins.
| } | ||
| ``` | ||
|
|
||
| This lets you use platform-specific features while maintaining a single codebase for notification publishing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth calling out explicitly that without Ably, you'd have to maintain 3 different platform-specific transformation layers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
|
||
| ## Device activation at scale | ||
|
|
||
| Before [devices](/docs/push/configure/device) or [browsers](/docs/push/configure/web) can receive push notifications, they must be activated - registered with both the platform's push service (APNs, FCM, or Web Push) and with Ably. Ably handles the complexity of maintaining these registrations at scale, including token refresh, mapping devices to users, and keeping track of subscriptions to channels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another key one to call out here is that we automatically handle stale registrations and exclude them from fanout, so you don't have to worry about stale devices causing delivery failures (e.g. due to inactivity, reinstalling app etc).
We can say something along the lines of (don't quote me, just an example) "at scale, stale registrations accumulate quickly, with Ably, this is entirely handled for you, with metachannels for monitoring key events in real time"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also handle automatically refreshing server credentials to talk to FCM/APNs etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Description
https://ably.atlassian.net/browse/FF-146
Checklist