Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log request ids in do_json_request_for on failure #1144

Closed
wants to merge 2 commits into from

Conversation

DMRobertson
Copy link
Contributor

@DMRobertson DMRobertson commented Sep 20, 2021

See corresponding synapse PR matrix-org/synapse#10854

@DMRobertson
Copy link
Contributor Author

I chose X-REQUEST-ID based on https://en.wikipedia.org/wiki/List_of_HTTP_header_fields#Common_non-standard_response_fields. It looks like that might be intended for client-generated ids rather than server-generated.

I didn't want to use a name that was synapse-specific in sytest though, e.g. what if dendrite or conduit want to expose a mechanism like this?

Could change synapse to only generate an id itself if the client hasn't provided a request id?

Another paranoid thought: is leaking the instance names and the id numbers a security risk? It's exposing internal details...

@DMRobertson
Copy link
Contributor Author

Closing. See also matrix-org/synapse#11090

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant