Looking at the commit history, it seems this was introduced by@ibetitsmike. Do you have any thoughts on how to proceed based on@johnstcn comment?

I'm just looking to fix the test race, but happy to hand this over to someone else if another approach is preferred.

@mafredri@johnstcn generally the only scenario in whichrequestLogger isnil is if the request circumvents our API Logger middleware. It's responsible for attachingRequestLogger to context.

So the only scenario where it fails is if we call the method that tries to log, but doesn't do it through API. An example that was failing for me was provisioner_job_tests which I fixed here:https://github.com/coder/coder/pull/17304/files#diff-a0b103dd31ee53f52c43564220dbada29c0d9147e7210b27cb42e375b08fd947

If we experience flakes I'd try to understand why a given test path fails to run this line:https://github.com/coder/coder/blob/5f7e5d709787ca4e6ec287197f112d56d0eae2c8/coderd/httpmw/loggermw/logger.go#L44C11-L44C28

RequestLogger is useful to debug issues with long lived connections, so it's not critical to have that information 100% of the time, but@deansheather was the one that initially asked me to remove thosenil checks (I had them in the first implementation).

Here's a repro:

go test ./coderd -run=Test_logFollower_completeBeforeFollow -v -race -count=10

There was another test as well that failed, also log follower. It's an easy miss to include the request logger when writing a test of this type, so I'm in favor of nil checks tbh as this happens too late and is racy otherwise.

What if we added another middleware only in tests that immediately fails the test if the request logging middleware is not present when any request comes in? That should allow surfacing this issue quickly.