Don't bother responding to client requests that have already disconnected

[anoadragon453]

This PR ports the quick fix from #2796 to further methods which handle media, URL preview and /key/v2/server requests. This prevents a harmless ERROR that comes up in the logs when we were unable to respond to a client request when the client had already disconnected. In this case we simply bail out if the client has already done so.

This is the 'simple fix' as suggested by #5304 (comment).

Fixes #6700
Fixes #5304

I was still seeing these in my logs, hence the quick PR for them.

[melroy89]

I still see this issue with the latest version of synapse. Is it really fixed??

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/twisted/internet/defer.py", line 1661, in _inlineCallbacks
    result = current_context.run(gen.send, result)
  File "/usr/local/lib/python3.8/site-packages/synapse/http/server.py", line 740, in _async_write_json_to_request_in_thread
    _write_bytes_to_request(request, json_str)
  File "/usr/local/lib/python3.8/site-packages/synapse/http/server.py", line 765, in _write_bytes_to_request
    _ByteProducer(request, bytes_generator)
  File "/usr/local/lib/python3.8/site-packages/synapse/http/server.py", line 565, in __init__
    self._request.registerProducer(self, True)
  File "/usr/local/lib/python3.8/site-packages/twisted/web/http.py", line 1043, in registerProducer
    self.channel.registerProducer(producer, streaming)
AttributeError: 'NoneType' object has no attribute 'registerProducer'

[anoadragon453]

@Danger89 Hmm, the only point where _async_write_json_to_request_in_thread is called is in respond_with_json, which was fixed in #2796 and should bail out early:

synapse/synapse/http/server.py

Lines 630 to 675 in 707d5e4

	def respond_with_json(
	request: SynapseRequest,
	code: int,
	json_object: Any,
	send_cors: bool = False,
	canonical_json: bool = True,
	):
	"""Sends encoded JSON in response to the given request.
	Args:
	request: The http request to respond to.
	code: The HTTP response code.
	json_object: The object to serialize to JSON.
	send_cors: Whether to send Cross-Origin Resource Sharing headers
	https://fetch.spec.whatwg.org/#http-cors-protocol
	canonical_json: Whether to use the canonicaljson algorithm when encoding
	the JSON bytes.
	Returns:
	twisted.web.server.NOT_DONE_YET if the request is still active.
	"""
	# could alternatively use request.notifyFinish() and flip a flag when
	# the Deferred fires, but since the flag is RIGHT THERE it seems like
	# a waste.
	if request._disconnected:
	logger.warning(
	"Not sending response to request %s, already disconnected.", request
	)
	return None
	if canonical_json:
	encoder = encode_canonical_json
	else:
	encoder = _encode_json_bytes
	request.setResponseCode(code)
	request.setHeader(b"Content-Type", b"application/json")
	request.setHeader(b"Cache-Control", b"no-cache, no-store, must-revalidate")
	if send_cors:
	set_cors_headers(request)
	run_in_background(
	_async_write_json_to_request_in_thread, request, encoder, json_object
	)
	return NOT_DONE_YET

Perhaps there's a rare race condition between the check at line no. 654 and the client disappearing just afterwards?

Could you please create a new issue with this traceback and roughly how often you are seeing this behaviour? We can continue discussion there.

[squahtx]

AttributeError: 'NoneType' object has no attribute 'registerProducer'

That particular bug was introduced in 1.44.0 (#10905) and should be fixed by #10995, which is in 1.45.0rc1.

... it seems we have two checks for disconnected clients now?

[anoadragon453]

@squahtx Thanks for the clarification!

... it seems we have two checks for disconnected clients now?

Four it looks like...

synapse/synapse/http/server.py

Lines 665 to 669 in daf498e

	if request._disconnected:
	logger.warning(
	"Not sending response to request %s, already disconnected.", request
	)
	return None

synapse/synapse/http/server.py

Lines 707 to 711 in daf498e

	if request._disconnected:
	logger.warning(
	"Not sending response to request %s, already disconnected.", request
	)
	return

synapse/synapse/http/server.py

Lines 809 to 813 in daf498e

	if request._disconnected:
	logger.warning(
	"Not sending response to request %s, already disconnected.", request
	)
	return

synapse/synapse/rest/media/v1/_base.py

Lines 241 to 245 in daf498e

	if request._disconnected:
	logger.warning(
	"Not sending response to request %s, already disconnected.", request
	)
	return