Repository: envoyproxy/envoy
File: source/common/router/router.cc (C++)

Code before:
  // Modify response headers after we have set the final upstream info because we may need to
  // modify the headers based on the upstream host.
  const Formatter::Context formatter_context(downstream_headers_, headers.get(), {}, {}, {},
                                             &callbacks_->activeSpan());
  route_entry_->finalizeResponseHeaders(*headers, formatter_context, callbacks_->streamInfo());
  modify_headers_(*headers);

  if (end_stream) {
    onUpstreamComplete(upstream_request);
  }

  callbacks_->encodeHeaders(std::move(headers), end_stream,
                            StreamInfo::ResponseCodeDetails::get().ViaUpstream);
}

void Filter::onUpstreamData(Buffer::Instance& data, UpstreamRequest& upstream_request,
                            bool end_stream) {
  // In normal situations, upstream_requests_ should have size 1 because when
  // we saw headers we either reset the stream (hence wouldn't have made it to
  // onUpstreamData) or all other in-flight streams.
  // There is one exception when route retry policy is configured and a HTTP
  // filter is in the upstream filter chain
  // and it's encodeHeaders() is returning FilterHeadersStatus::StopIteration.
  // In this case, onUpstreamData() might get called with
  // upstream_requests_.size() == 0 and we should just return.
  if (upstream_requests_.size() != 1) {
    return;
  }

  if (end_stream) {
    // gRPC request termination without trailers is an error.
    if (upstream_request.grpcRqSuccessDeferred()) {
      upstream_request.upstreamHost()->stats().rq_error_.inc();
    }
    onUpstreamComplete(upstream_request);
  }

  callbacks_->encodeData(data, end_stream);
}

void Filter::onUpstreamTrailers(Http::ResponseTrailerMapPtr&& trailers,
                                UpstreamRequest& upstream_request) {
  // In normal situations, upstream_requests_ should have size 1 because when
  // we saw headers we either reset the stream (hence wouldn't have made it to
  // onUpstreamTrailers) or all other in-flight streams.
  // There is one exception when route retry policy is configured and a HTTP
  // filter is in the upstream filter chain
  // and it's encodeHeaders() is returning FilterHeadersStatus::StopIteration.
  // In this case, onUpstreamTrailers() might get called with
  // upstream_requests_.size() == 0 and we should just return.
  if (upstream_requests_.size() != 1) {

Review comment (suggestion):
The original assertion implies the value must be 1, whereas the comment says it could be 0 or 1.
If that's the case, and assuming that 0 is a valid value, then it may be better to do the following:
```
// Explain when the size can be 0.
if (upstream_requests_.size() == 0) {
  // Consider adding another assertion that captures when this can occur.
  return;
}
// Explain why the size can only be 1 at this point.
ASSERT(upstream_requests_.size() == 1);
```

Code after:
  }

  callbacks_->streamInfo().setResponseCodeDetails(
      StreamInfo::ResponseCodeDetails::get().ViaUpstream);

  callbacks_->streamInfo().setResponseCode(response_code);
  downstream_response_started_ = true;
  final_upstream_request_ = &upstream_request;
  // Make sure that for request hedging, we end up with the correct final upstream info.
  callbacks_->streamInfo().setUpstreamInfo(final_upstream_request_->streamInfo().upstreamInfo());
  resetOtherUpstreams(upstream_request);

  // Modify response headers after we have set the final upstream info because we may need to
  // modify the headers based on the upstream host.
  const Formatter::Context formatter_context(downstream_headers_, headers.get(), {}, {}, {},
                                             &callbacks_->activeSpan());
  route_entry_->finalizeResponseHeaders(*headers, formatter_context, callbacks_->streamInfo());
  modify_headers_(*headers);

  if (end_stream) {
    onUpstreamComplete(upstream_request);
  }

  callbacks_->encodeHeaders(std::move(headers), end_stream,
                            StreamInfo::ResponseCodeDetails::get().ViaUpstream);
}

void Filter::onUpstreamData(Buffer::Instance& data, UpstreamRequest& upstream_request,
                            bool end_stream) {
  // When route retry policy is configured and an upstream filter is returning StopIteration
  // in it's encodeHeaders() method, upstream_requests_.size() is equal to 0 in this case,
  // and we should just return.
  if (upstream_requests_.size() == 0) {
    return;
  }

  // Other than above case, this should be true because when we saw headers we
  // either reset the stream (hence wouldn't have made it to onUpstreamData) or
  // all other in-flight streams.
  ASSERT(upstream_requests_.size() == 1);
  if (end_stream) {
    // gRPC request termination without trailers is an error.
    if (upstream_request.grpcRqSuccessDeferred()) {
      upstream_request.upstreamHost()->stats().rq_error_.inc();
    }
    onUpstreamComplete(upstream_request);
  }

  callbacks_->encodeData(data, end_stream);
}