Skip to content

Revert "Fix flaky 3PID inhibit error unit tests"#19916

Open
anoadragon453 wants to merge 3 commits into
developfrom
revert-19913-anoa/fix_flaky_3pid_tests
Open

Revert "Fix flaky 3PID inhibit error unit tests"#19916
anoadragon453 wants to merge 3 commits into
developfrom
revert-19913-anoa/fix_flaky_3pid_tests

Conversation

@anoadragon453

@anoadragon453 anoadragon453 commented Jul 7, 2026

Copy link
Copy Markdown
Member

This PR reverts #19913, but leaves behind the comments in the tests noting that the endpoints have jitter.

See #19913 (comment) as for why. Kudos to @MadLittleMods for digging into the issue properly and providing an elegant fix in #19879!


928716e is the only new code here.

@anoadragon453 anoadragon453 marked this pull request as ready for review July 7, 2026 09:52
@anoadragon453 anoadragon453 requested a review from a team as a code owner July 7, 2026 09:52
Comment on lines +329 to +333
# Note: The endpoint intentionally adds up to 1000ms of jitter to avoid
# leaking whether the email address is bound to an account.
#
# This should be handled by `await_result` underneath, which has a
# 1000ms timeout.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could move this next to the relevant make_request inside _request_token(...)

Comment on lines +752 to 761
# Note: The endpoint intentionally adds up to 1000ms of jitter to avoid
# leaking whether the email address is bound to an account.
#
# This should be handled by `await_result` underneath, which has a
# 1000ms timeout.
channel = self.make_request(
"POST",
b"register/email/requestToken",
{"client_secret": "foobar", "email": email, "send_attempt": 1},
# The endpoint intentionally adds up to 1000ms of jitter to avoid
# leaking whether the email address is already bound to an account.
timeout_ms=3000,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current comment works.

I could also see this explicit version being good:

Suggested change
# Note: The endpoint intentionally adds up to 1000ms of jitter to avoid
# leaking whether the email address is bound to an account.
#
# This should be handled by `await_result` underneath, which has a
# 1000ms timeout.
channel = self.make_request(
"POST",
b"register/email/requestToken",
{"client_secret": "foobar", "email": email, "send_attempt": 1},
# The endpoint intentionally adds up to 1000ms of jitter to avoid
# leaking whether the email address is already bound to an account.
timeout_ms=3000,
)
channel = self.make_request(
"POST",
b"register/email/requestToken",
{"client_secret": "foobar", "email": email, "send_attempt": 1},
await_result=False,
)
# Note: The endpoint intentionally adds up to 1000ms of jitter to avoid
# leaking whether the email address is bound to an account.
channel.await_result(timeout_ms=1000)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants