comparison mercurial/wireprotov2peer.py @ 40724:15a643304728 stable

wireprotov2peer: wait for initial object before resolving future As part of rolling out wireprotov2 with redirect support, I encountered an edge case with regards to future resolution. Essentially, the initial response frame from the server did not fully decode the initial CBOR object. The frame wasn't marked as EOS. In the previous code, we resolved the future for the request to response.objects(), which mapped to the commandresponse instance which would eventually produce a redirect. Upon receiving subsequent data, the initial CBOR object containing the redirect would be decoded and we'd process the redirect. However, the future would already have been resolved with the initial commandresponse.objects() and the client iterating over the objects wouldn't receive any objects from the redirect because the redirect was populating a different commandresponse instance! This commit changes the logic so we don't resolve futures until the initial CBOR response object is fully decoded or until EOS occurs. In cases where there is an empty or partial frame associated with a redirect, the future will now resolve with the commandresponse containing the proper series of decoded objects.
author Gregory Szorc <gregory.szorc@gmail.com>
date Wed, 28 Nov 2018 12:52:23 -0800
parents 94b0d0f996e1
children e6c1c6478d04
comparison
equal deleted inserted replaced
40723:94b0d0f996e1 40724:15a643304728
375 375
376 def _processresponsedata(self, frame, meta, response): 376 def _processresponsedata(self, frame, meta, response):
377 # This can raise. The caller can handle it. 377 # This can raise. The caller can handle it.
378 response._onresponsedata(meta['data']) 378 response._onresponsedata(meta['data'])
379 379
380 # If we got a content redirect response, we want to fetch it and 380 # We need to be careful about resolving futures prematurely. If a
381 # expose the data as if we received it inline. But we also want to 381 # response is a redirect response, resolving the future before the
382 # keep our internal request accounting in order. Our strategy is to 382 # redirect is processed would result in the consumer seeing an
383 # basically put meaningful response handling on pause until EOS occurs 383 # empty stream of objects, since they'd be consuming our
384 # and the stream accounting is in a good state. At that point, we follow 384 # response.objects() instead of the redirect's response.objects().
385 # the redirect and replace the response object with its data. 385 #
386 386 # Our strategy is to not resolve/finish the request until either
387 redirect = response._redirect 387 # EOS occurs or until the initial response object is fully received.
388 handlefuture = False if redirect else True 388
389 389 # Always react to eos.
390 if meta['eos']: 390 if meta['eos']:
391 response._oninputcomplete() 391 response._oninputcomplete()
392 del self._requests[frame.requestid] 392 del self._requests[frame.requestid]
393 393
394 if redirect: 394 # Not EOS but we haven't decoded the initial response object yet.
395 self._followredirect(frame.requestid, redirect) 395 # Return and wait for more data.
396 return 396 elif not response._seeninitial:
397 397 return
398 if not handlefuture: 398
399 # The specification says no objects should follow the initial/redirect
400 # object. So it should be safe to handle the redirect object if one is
401 # decoded, without having to wait for EOS.
402 if response._redirect:
403 self._followredirect(frame.requestid, response._redirect)
399 return 404 return
400 405
401 # If the command has a decoder, we wait until all input has been 406 # If the command has a decoder, we wait until all input has been
402 # received before resolving the future. Otherwise we resolve the 407 # received before resolving the future. Otherwise we resolve the
403 # future immediately. 408 # future immediately.