Page MenuHomePhabricator

[services] Tunnelbroker - Adding of requesting a new notification token if empty
ClosedPublic

Authored by max on Nov 4 2022, 9:29 AM.
Tags
None
Referenced Files
F3509968: D5536.diff
Sat, Dec 21, 9:50 AM
Unknown Object (File)
Fri, Dec 13, 1:35 AM
Unknown Object (File)
Wed, Dec 4, 3:30 AM
Unknown Object (File)
Nov 19 2024, 8:54 PM
Unknown Object (File)
Nov 19 2024, 8:54 PM
Unknown Object (File)
Oct 29 2024, 11:29 AM
Unknown Object (File)
Oct 25 2024, 10:09 PM
Unknown Object (File)
Oct 2 2024, 8:31 AM

Details

Summary

This diff introduces the handling of requesting the new notification token from the client in case if the token is empty.
When the Tunnelbroker sends a push notification to the device, the notification token can be wrong or expired.
In this case, we are making the token empty for a certain device/session and requesting the new token from the client using the NewNotifyTokenRequired message.
Handling of the new token from the client in D5534.

Linear task: ENG-2060

Test Plan
  • Connect to the bidirectional gRPC MessagesStream stream with the valid sessionID in metadata and send the message to the server with the NewNotifyToken field value as a random string. The expected behavior is that the value will be written into the sessions table of the dynamoDB database into the NotifyToken field;
  • Make the NotifyToken field empty in a record with your sessionID key;
  • Re-connect to the bidirectional gRPC MessagesStream;

The expected behavior is your client will receive a newNotifyTokenRequired message to require the new token from the device which must be sent as NewNotifyToken message and will be handled in the D5534 diff.

Diff Detail

Repository
rCOMM Comm
Branch
D5536
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

max held this revision as a draft.
max published this revision for review.Nov 4 2022, 9:55 AM
max edited the summary of this revision. (Show Details)
max edited the test plan for this revision. (Show Details)
max added reviewers: varun, marcin. max added 1 blocking reviewer(s): jon.
services/tunnelbroker/src/server/mod.rs
133 ↗(On Diff #18086)

I know usage of strings was already brought up, but I think we should try to use an enum on the rust side. Not a blocker for this PR, but moving forward it will be nice to get compile time errors, rather than runtime errors when a string might have invalid contents (e.g. "MOBLIE")

Similar to https://linear.app/comm/issue/ENG-1755/avoid-stringly-typed-values

145 ↗(On Diff #18086)

would like to avoid usage of stderr and use log tracing

services/tunnelbroker/src/server/mod.rs
101–105 ↗(On Diff #18086)

Changes to use tracing debug! instead of eprintln!.
Matching gRPC enum using the integer representation instead of string matching.

max added inline comments.
services/tunnelbroker/src/server/mod.rs
133 ↗(On Diff #18086)

I know usage of strings was already brought up, but I think we should try to use an enum on the rust side. Not a blocker for this PR, but moving forward it will be nice to get compile time errors, rather than runtime errors when a string might have invalid contents (e.g. "MOBLIE")

Similar to https://linear.app/comm/issue/ENG-1755/avoid-stringly-typed-values

After changing the field type to integer in D5560 we can now match the enum with the integer representation instead of string matching.

145 ↗(On Diff #18086)

would like to avoid usage of stderr and use log tracing

I've changed it. Thx.

This revision is now accepted and ready to land.Nov 9 2022, 11:02 AM
This revision now requires review to proceed.Nov 10 2022, 2:15 AM
This revision is now accepted and ready to land.Nov 10 2022, 8:45 AM
max marked 2 inline comments as done.

Rebasing on parent and master changes.

This revision was landed with ongoing or failed builds.Nov 21 2022, 5:31 AM
This revision was automatically updated to reflect the committed changes.