Track invites and allow cancelling invites #447
Comments
gmaclennan
changed the title
|
After reviewing the implementation of invites in the front-end, it seems like we need a way to cancel invites. If a user invites another device, they are currently left waiting for a response, and need a way to cancel if they do not receive a reply within an expected time-frame. The cancellation should ensure that when a response is received, it is ignored (currently whenever a response to an invite is received, the invited device is automatically added to the project). The cancellation should also send a wire message to the invitee, so that they can see that their invite has been cancelled (and this should result in the project key to be deleted, and the project instance to be deleted if it has not already been). This points to perhaps the need for an additional step in the invite flow, of the invitee receiving a final message back about whether they were actually added to a project. |
|
Here's a proposal for how we'd do this. tl;drCurrently, two messages are sent:
I propose that three messages be sent:
Made a little diagram: sequenceDiagram
Inviter->>Invitee: Invite (limited info)
Invitee->>Inviter: Accept
Inviter->>Invitee: Project details (key, encryption keys, etc)
GoalsHere are the goals of the invite system (I think):
Non-goals:
Proposed user experienceHappy path:
Happy path where two people invite the same person:
Unhappy path where invite is canceled:
Unhappy path where invite is rejected:
Unhappy path where the user is already part of the project:
Unhappy path where connection is lost before invite can be sent:
Unhappy path where connection is lost before invite can be sent:
Proposed front-end code
Proposed back-end codeHigh-level overview: sequenceDiagram
Inviter->>Invitee: Invite (limited info)
Invitee->>Inviter: Accept
Inviter->>Invitee: Project details (key, encryption keys, etc)
Here's a sketch of the inviter code: export class MemberApi {
// ...
async invite(
deviceId: string,
{ roleId, signal }: { roleId: string; signal: AbortSignal },
): Promise<InviteResponse_Decision> {
// Set up our abort signals.
const timeoutSignal = AbortSignal.timeout(60_000);
const abortSignal = AbortSignal.any([signal, timeoutSignal]);
const handleAborted = () => {
if (abortSignal.aborted) {
throw new ProjectInviteError(
(abortSignal.reason?.name === "TimeoutError") ? "TIMEOUT" : "ABORTED",
);
}
};
// Was this aborted before we even started?
handleAborted();
// Find the peer.
let peer: Peer;
try {
peer = await this.#getPeerByDeviceId(deviceId);
} catch (err) {
handleAborted();
console.error("Couldn't find peer to invite", err);
throw new ProjectInviteError("PEER_NOT_FOUND");
}
// Create a cleanup function.
const cleanUp = () => {
peer.pendingInvites.delete(this.#projectId);
};
// Do we already have a pending invite for this device?
if (peer.pendingInvites.has(this.#projectId)) {
throw new ProjectInviteError("DUPLICATE_INVITE");
}
// Make sure we clean up if we abort.
once(abortSignal, "abort", cleanUp);
// Add the pending invite.
//
// Do this BEFORE we send anything because it's possible (though unlikely) that we'll
// get a response before we believe it's sent.
const deferred = Promise.withResolvers<InviteResponse_Decision>();
peer.pendingInvites.set(this.#projectId, deferred);
// Send the invite.
peer.sendInvite({ roleId });
// Wait for their decision. Make sure we clean up.
let decision: InviteResponse_Decision;
try {
decision = await deferred.promise;
} finally {
peer.pendingInvites.delete(this.#projectId);
}
handleAborted();
// Respond to the decision. At this point, we can't cancel.
switch (decision) {
case "ACCEPT":
await peer.sendProjectInfo({ projectKey, encryptionKeys });
addPeerToProject();
return decision;
case "ALREADY":
case "REJECT":
return decision;
case "UNRECOGNIZED":
console.warn("Got an unrecognized decision.");
return decision;
default:
throw new ShouldBeImpossibleError(decision);
}
}
// ...
}When the invitee receives the invite message, UI should be shown. When the invitee receives the project info message, they should join the project without user intervention. |
|
Great, thanks for this, really helpful to have this written up. Suggestions for goals:
The key thing we are hiding from the front-end is inviting multiple peers at the same time. The UI is currently designed for a serial invite process: invite peer A -> invite accepted -> invite peer B -> invite accepted -> invite peerC etc. The reason for this is to avoid extra complexity in the user experience (handling pending invites managing them) at the expense of inconvenience when inviting multiple people (most common in a training environment). We want to gather more feedback on partners about this when we role it out. But yes, the backend should be able to handle multiple active invites to the same project being "pending" at the same time, but we should not allow it (e.g. throw when trying to send an invite when there is already a pending invite).
Yes, we can't do anything to avoid this. Different devices could independently decide to invite a peer on the network.
Strictly, users only respond "accept" or "reject". "already" is automatic and without user intervention.
What we want to do, at this stage, is avoid the user concept (in the UX) that there can be more than one pending invite and it is "in the background". The user experience we currently want is that waiting for an invite response is an active process that involves waiting on a screen, and in-person prompting of the invited device to respond.
I'm wondering if we should always have this, and show it to the device receiving the invite (e.g. a countdown timer on the invite). I think probably out-of-scope for this initial implementation (the UX part), but I think we should add TTL functionality.
Non-goals:
I think we should auto-cancel invites when the app is closed / put into the background. I think we should also auto-cancel invites when the connection to the peer is lost.
What is in scope (and is currently what happens) is that the invitee only sees the first invite they received, and that is the only one they respond to. All other invites (with or without conflicting roles) receive either "reject" or "already" as the response.
Inviter A sees that their invite is accepted
Invitee sees that their response was accepted, and joins the project.
Inviter B sees that the invitee is already a member of the project (does not happen until invitee responds to A)
I think the invitee needs to know about the cancellation immediately, otherwise the invitor would not be able to re-send the invite (because as a duplicate invite to the same project, it would be ignored by the invitee).
The invitee should not see anything in this path. I think this maybe meant to say "Inviter" in (2). However the inviter should see that the response is "already part of the project" as distinct from "accepted".
Ideally we detect connection loss and immediately cancel the invite.
Not sure about this one, e.g. the re-try.
Just a note that we cannot pass an abort signal across the IPC channel. We can create this method on the front-end by overriding the
I think maybe Additional thoughts
|
Good catch, that was a typo.
I didn't know this—you probably saved me at least hour of confusion!
I agree that it doesn't matter much. When I implement this, I'll do whatever's easiest (unless I divine some strong reason why one option is preferable).
👍️
👍️
I'll file two separate post-MVP issues for these...does that sound reasonable? |
|
Started working on this. The overall happy path, I think:
I think these are the protos: message Invite {
bytes inviteId = 1;
string projectPublicId = 2;
string projectName = 3;
optional string roleName = 4;
optional string roleDescription = 5;
optional string invitorName = 6;
}
message InviteCancel {
bytes inviteId = 1;
}
message InviteResponse {
enum Decision {
REJECT = 0;
ACCEPT = 1;
ALREADY = 2;
}
bytes responseToInviteId = 1;
Decision decision = 2;
}
message ShareProjectDetails {
bytes inviteId = 1;
bytes projectKey = 2;
EncryptionKeys encryptionKeys = 3;
}Here's the happy path: sequenceDiagram
Inviter->>Invitee: Invite
Invitee->>Inviter: InviteResponse with ACCEPT
Inviter->>Invitee: ShareProjectDetails
Still working on this so things might change! |
|
Spoke with Erik and we decided to have an explicit "cancel invite" method instead of an abort controller. Should be conceptually the same, but a small deviation from the current public API. |
|
Looks good! quick thoughts:
|
|
#484 does most of the work here. Most significantly, invite cancellations are missing. Here's what I think that entails...This assumes that #484 doesn't change drastically. We'd add a new RPC message: message InviteCancel {
bytes inviteId = 1;
}We'd add a method on /**
* @param {string} deviceId
* @param {InviteCancel} inviteCancel
* @returns {Promise<void>}
*/
async sendInviteCancel(deviceId, inviteCancel) {
await this.#waitForPendingConnections()
const peer = await this.#getPeerByDeviceId(deviceId)
peer.sendInviteCancel(inviteCancel)
}We'd add the ability to send cancelations in /** @param {InviteCancel} inviteCancel */
sendInviteCancel(inviteCancel) {
this.#assertConnected()
const buf = Buffer.from(InviteCancel.encode(inviteCancel).finish())
const messageType = MESSAGE_TYPES.InviteCancel
this.#channel.messages[messageType].send(buf)
this.#log('sent invite cancel %h', inviteCancel.inviteId)
}Then, in case 'InviteCancel': {
const inviteCancel = InviteCancel.decode(value)
if (!isInviteCancelValid(inviteCancel)) {
this.#l.log('Received invalid invite cancel')
return
}
const peerId = keyToId(protomux.stream.remotePublicKey)
this.emit('invite-cancel', peerId, inviteCancel)
this.#l.log(
'Invite cancel from %S for %h',
peerId,
inviteCancel.inviteId
)
break
}
// ...
/**
* @param {InviteCancel} inviteCancel
* @returns {boolean}
*/
function isInviteCancelValid(inviteCancel) {
return isInviteIdValid(inviteCancel.inviteId)
}Finally, we would need to:
|
See [#447][0] for details. [0]: #447 Co-authored-by: Gregor MacLennan <[email protected]>
This PR adds invite cancelations. Notable changes: - `InviteCancel` RPC messages are now sent and received. - `project.$member.invite()` takes an `AbortSignal`, used for cancelations. It replaces the timeout option. - The `invite-removed` event now has a "reason" parameter which the frontend can use. Closes #447.
A peer might have sent an invite and then want to expire it manually, or change the invite (e.g. re-invite) with different role positions. This would require the receiver to:
A) Override a previously received invite with the new one
B) Include some kind of identifier in the response to the the invitor know which invite is being responded to
The text was updated successfully, but these errors were encountered: