[keyserver] Don't block innerPerformAuth return on sleep
ClosedPublic
Actions

Authored by ashoat on Mar 31 2024, 7:45 PM.

Details

Reviewers

tomek
inka

Commits

rCOMMeb1952d00f4b: [keyserver] Don't block innerPerformAuth return on sleep

Summary

In an earlier version of this stack, in innerPerformRecovery I would fall back on legacy login, and without this change the fallback would have to wait on this sleep.

This didn't used to matter before because nothing actually waits for innerPerformAuth to complete. And since I eventually came to the conclusion that the fallback was a bad idea (we'd be sending user passwords to keyservers), it actually still doesn't matter.

I decided to put this diff up anyways because I think it's probably better for us to avoid blocking the return of innerPerformAuth on this sleep.

Depends on D11500

Test Plan

Confirm in the earlier version of the stack that when falling back on legacy login, there was no delay due to this sleep

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ashoat created this revision.Mar 31 2024, 7:45 PM

ashoat added a child revision: D11502: [lib] Make invalidSessionRecovery work with all recoveryActionSources handled by KeyserverConnectionHandler.

Harbormaster completed remote builds in B27881: Diff 38614.Mar 31 2024, 8:03 PM

ashoat requested review of this revision.Mar 31 2024, 8:03 PM

tomek accepted this revision.Apr 2 2024, 2:47 AM

tomek added inline comments.

lib/components/keyserver-connection-handler.js
182–185	It seems surprising to me that this sleep was blocking the return from `innerPerformAuth` - this code is part of a promise created on line 114, so it should immediately return. Maybe the reason is to avoid waiting for sleep to complete when awaiting the result of calling `innerPerformAuth`?

This revision is now accepted and ready to land.Apr 2 2024, 2:47 AM

ashoat added inline comments.Apr 2 2024, 6:48 AM

lib/components/keyserver-connection-handler.js
182–185	Yes, it's about a callsite where we `await` the result of calling `innerPerformAuth`. I've highlighted it below
270–275	The issue is that this promise will not resolve until the `sleep` concludes