This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Abort federation requests if the client disconnects early #7930
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For inbound federation requests, if a given remote server makes too many requests at once, we start stacking them up rather than processing them immediatedly. However, that means that there is a fair chance that the requesting server will disconnect before we start processing the request. In that case, if it was a read-only request (ie, a GET request), there is absolutely no point in building a response (and some requests are quite expensive to handle). Even in the case of a POST request, one of two things will happen: * Most likely, the requesting server will retry the request and we'll get the information anyway. * Even if it doesn't, the requesting server has to assume that we didn't get the memo, and act accordingly. In short, we're better off aborting the request at this point rather than ploughing on with what might be a quite expensive request.
clokep
approved these changes
Jul 22, 2020
| "client disconnected before we started processing " | ||
| "request" | ||
| ) | ||
| return -1, None |
There was a problem hiding this comment.
-1 just because this is a garbage response and it doesn't really matter?
This seems similar to #7927 where 200, {} was returned. Why the difference?
There was a problem hiding this comment.
errr good question.
None ensures that we don't call respond_with_json.
There was a problem hiding this comment.
Should we be using it in the other place too then? 😄 Either way I think this is OK!
There was a problem hiding this comment.
yes, probably. I think this this the better way. we could clean up #7927 for consistency but I don't think it's a big deal really.
babolivier
pushed a commit
that referenced
this pull request
Sep 1, 2021
* commit '4876af06d': Abort federation requests if the client disconnects early (#7930)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For inbound federation requests, if a given remote server makes too many
requests at once, we start stacking them up rather than processing them
immediatedly.
However, that means that there is a fair chance that the requesting server will
disconnect before we start processing the request. In that case, if it was a
read-only request (ie, a GET request), there is absolutely no point in
building a response (and some requests are quite expensive to handle).
Even in the case of a POST request, one of two things will happen:
Most likely, the requesting server will retry the request and we'll get the
information anyway.
Even if it doesn't, the requesting server has to assume that we didn't get
the memo, and act accordingly.
In short, we're better off aborting the request at this point rather than
ploughing on with what might be a quite expensive request.