Skip to content

Conversation

@code-asher
Copy link
Member

@code-asher code-asher commented Nov 14, 2025

I am trying to debug coder/internal#1010 and coder/internal#1059 where I get a context timeout on API requests to the agent.

So, I thought maybe it could make sense to log agent API requests to see if it is even getting the requests, along with adding the recovery middleware because maybe it is panicking.

Because rbac cannot be imported in slim, I had to split the logger middleware into slim and non-slim versions.

And similarly, exclude other middleware that will not make sense in a slim context (mostly because they import rbac or database).

Although, I wonder if I should just break out the middleware I need (recover, loggermw, and tracing right now) into a new package because most of httpmw seems to be designed around being used in coderd specifically and will probably never make sense in a slim context. Would a top-level httpmw package make sense maybe?

@Emyrk tagged you just because I saw a comment elsewhere about possibly creating a new package for middleware

@code-asher code-asher changed the title chore: add debug logging to agent api requests chore: add debug logging and recovery to agent api requests Nov 14, 2025
@code-asher code-asher force-pushed the asher/agent-api-logs branch 5 times, most recently from 7b05da2 to 3b15e9e Compare November 18, 2025 23:20
@code-asher code-asher requested a review from Emyrk November 19, 2025 19:54
@Emyrk
Copy link
Member

Emyrk commented Nov 19, 2025

Although, I wonder if I should just break out the middleware I need (recover, loggermw, and tracing right now) into a new package because most of httpmw seems to be designed around being used in coderd specifically and will probably never make sense in a slim context. Would a top-level httpmw package make sense maybe?

Yea, this is preferred over build tags in the httpmw package as a whole imo. That is very easy for someone to mess up when they add a new mw.

@code-asher code-asher force-pushed the asher/agent-api-logs branch 3 times, most recently from ceb48d7 to 89b4c8e Compare November 20, 2025 00:25
@code-asher
Copy link
Member Author

I ended up just moving recover because moving the others made a bigger diff than I expected, I can def move them if we want but I went with the minimal impact for now.

@code-asher code-asher merged commit c266bb8 into main Nov 25, 2025
30 checks passed
@code-asher code-asher deleted the asher/agent-api-logs branch November 25, 2025 23:59
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants