Limit health endpoint to /health$
#19405
Conversation
| request.setResponseCode(404) | ||
| body = ( | ||
| '{"errcode":"' | ||
| + Codes.UNRECOGNIZED | ||
| + '","error":"Unrecognized request"}' | ||
| ) |
There was a problem hiding this comment.
I wasn't able to use a UnrecognizedRequestError here directly as no calling function would catch and convert it. And I didn't want to hardcode M_UNRECOGNIZED in case it ever changed.
I also didn't want to construct an object and then convert that to JSON, as the health endpoint is supposed to be fast.
There was a problem hiding this comment.
This seems like a good approach to keep the /health endpoint logic as simple as possible and speedy.
devonh
left a comment
devonh
left a comment
There was a problem hiding this comment.
Thank you for taking this on so quickly and just getting it out of the way :)
| request.setResponseCode(404) | ||
| body = ( | ||
| '{"errcode":"' | ||
| + Codes.UNRECOGNIZED | ||
| + '","error":"Unrecognized request"}' | ||
| ) |
There was a problem hiding this comment.
This seems like a good approach to keep the /health endpoint logic as simple as possible and speedy.
| """ | ||
| channel = self.make_request("GET", "/health/extra/path", shorthand=False) | ||
|
|
||
| self.assertEqual(channel.code, 404) |
There was a problem hiding this comment.
Should we also test the error body for completeness sake?
Similar to what we do for testing generic server requests
| self.assertEqual(channel.code, 404) | |
| self.assertEqual(channel.code, 404) | |
| self.assertEqual(channel.json_body["error"], "Unrecognized request") | |
| self.assertEqual(channel.json_body["errcode"], "M_UNRECOGNIZED") |
Fixes #19395 by checking for
/healthexactly (Twisted didn't appear to make it easy to use a regex here - and an exact str check is quicker than a regex match anyhow).Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.