Page MenuHomePhabricator

[lib] implement sending APNs notifs using Tunnelbroker
ClosedPublic

Authored by kamil on Jun 28 2024, 5:51 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 8:44 PM
Unknown Object (File)
Fri, Nov 22, 4:48 PM
Unknown Object (File)
Tue, Nov 12, 2:30 PM
Unknown Object (File)
Tue, Nov 12, 2:29 PM
Unknown Object (File)
Tue, Nov 12, 2:25 PM
Unknown Object (File)
Tue, Nov 12, 12:23 PM
Unknown Object (File)
Tue, Nov 12, 6:30 AM
Unknown Object (File)
Sun, Oct 27, 2:06 PM

Details

Summary

Client code to send notif.

Depends on D12616

Test Plan
try {
     const notif: TunnelbrokerAPNsNotif = {
       type: 'APNsNotif',
       headers: JSON.stringify({ 'apns-topic': 'app.comm' }),
       clientMessageID: uuid.v4(),
       deviceID: '[my device ID]',
       payload:
         '{"aps":{"alert":{"title":"your_title_here","body":"your_body_here"},"mutable-content":1}}',
     };
     await sendNotif(notif);
   } catch (e) {
     console.log(e);
   }

Testing this code multiple times with different TB configs and different headers.

Note: Not testing different payloads, code to create that is now being moved from keyserver so assuming it should work, Tunnelbroker does not modify notif itself. (cc. @marcin)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Jun 28 2024, 6:35 AM
lib/tunnelbroker/tunnelbroker-context.js
359–365 ↗(On Diff #41795)

What is the point of all of this?

Why can't we just use sendMessageToDeviceRequest directly?

If it's a type issue, can we fix the type issue directly instead of making a duplicate function?

remove callback

lib/tunnelbroker/tunnelbroker-context.js
359–365 ↗(On Diff #41795)

Initially, I wanted the param here to be APNsNotification, and based on that to build TunnelbrokerAPNsNotif (I wanted to avoid forcing building Tunnelbroker specific types when calling), but I was waiting to rebase this on D12429.

However, I discussed this today with @marcin and it'll be more convenient to leave it as it is right now - that being said we can use sendMessageToDeviceRequest directly.

lib/tunnelbroker/tunnelbroker-context.js
359–365 ↗(On Diff #41795)

additionally, listed point 1 and 2 in ENG-8621 to improve naming here

bartek added inline comments.
lib/tunnelbroker/tunnelbroker-context.js
359–365 ↗(On Diff #41795)

Isn't there a way to alias sendMessageToDeviceRequest()? Sth like

const sendNotif: (notif: TunnelbrokerAPNsNotif) => Promise<void>
  = sendMessageToDeviceRequest;

Edit: ah, that's what you did in the next revision 😉

This revision is now accepted and ready to land.Jul 2 2024, 3:41 AM