Skip to content

Conversation

@anoadragon453
Copy link
Member

@anoadragon453 anoadragon453 commented Aug 1, 2025

Reported by @realtyem in DMs. The consequence was that logcontexts became lost for these code paths:

synapse.http.server - 129 - INFO - sentinel - <SynapseRequest at 0x747656210a50 method='POST' uri='/_matrix/client/r0/account/password/email/requestToken' clientproto='1.1' site='test'> SynapseError: 429 - Too Many Requests (rc_3pid_validation)
synapse.logging.context - 1003 - WARNING - sentinel - Calling defer_to_threadpool from sentinel context: metrics will be lost
synapse.logging.context - 91 - WARNING - sentinel - Expected logging context POST-458 was lost

Missed in #18546 and #18595.

cc @erikjohnston

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

@anoadragon453 anoadragon453 marked this pull request as ready for review August 1, 2025 12:06
@anoadragon453 anoadragon453 requested a review from a team as a code owner August 1, 2025 12:06
@anoadragon453 anoadragon453 enabled auto-merge (squash) August 1, 2025 12:06
@erikjohnston
Copy link
Member

"oops" 😬

@anoadragon453
Copy link
Member Author

CI failures are a flaky test. Merging manually.

@anoadragon453 anoadragon453 disabled auto-merge August 1, 2025 15:00
@anoadragon453 anoadragon453 merged commit 510924a into develop Aug 1, 2025
42 of 44 checks passed
@anoadragon453 anoadragon453 deleted the anoa/await_sleeps branch August 1, 2025 15:00
except LimitExceededError as e:
if e.pause:
self._clock.sleep(e.pause)
await self._clock.sleep(e.pause)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something we can lint?

We already detect this kind of thing for async/await

error: Value of type "Coroutine[Any, Any, None]" must be used  [unused-coroutine]
note: Are you missing an await?

Copy link
Member

Choose a reason for hiding this comment

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

Oh mmm, we probably want to change the return type of sleep

Copy link
Member

Choose a reason for hiding this comment

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

I did this here: #18772

erikjohnston added a commit that referenced this pull request Aug 4, 2025
This helps ensure that mypy can catch places where we don't await on it,
like in #18763.
erikjohnston added a commit that referenced this pull request Aug 5, 2025
This helps ensure that mypy can catch places where we don't await on it,
like in #18763.

---------

Co-authored-by: Eric Eastwood <[email protected]>
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.

5 participants