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

Authored by • max on Nov 4 2022, 9:29 AM.

Details

Reviewers

• jon
varun
marcin
tomek

Commits

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

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• max created this revision.Nov 4 2022, 9:29 AM

• max held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptNov 4 2022, 9:29 AM

Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald Transcript

• max added a parent revision: D5535: [services] Tunnelbroker - Removing of emptiness check for notification token.Nov 4 2022, 9:31 AM

Harbormaster completed remote builds in B13180: Diff 18086.Nov 4 2022, 9:41 AM

• max mentioned this in D5535: [services] Tunnelbroker - Removing of emptiness check for notification token.Nov 4 2022, 9:49 AM

• 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.

• jon added inline comments.Nov 4 2022, 11:05 AM

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

• max added a child revision: D5540: [services] Tunnelbroker - Adding delivering of undelivered messages from the database to the messages stream.Nov 6 2022, 6:09 AM

• max added a parent revision: D5534: [services] Tunnelbroker - Adding of handling of the `NewNotifyToken` message.Nov 7 2022, 7:06 AM

• max mentioned this in rCOMM7d3f184567a7: [services] Tunnelbroker - Removing of emptiness check for notification token.Nov 7 2022, 7:32 AM

varun added inline comments.Nov 7 2022, 7:58 AM

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

• max removed a parent revision: D5535: [services] Tunnelbroker - Removing of emptiness check for notification token.Nov 7 2022, 9:04 AM

• max planned changes to this revision.Nov 8 2022, 8:05 AM

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

• max marked 3 inline comments as done.Nov 8 2022, 3:56 PM

• 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.