Repository: envoyproxy/envoy
File: source/extensions/filters/http/stateful_session/stateful_session.h (C/C++)

Code before:
  StatefulSession(StatefulSessionConfigSharedPtr config) : config_(std::move(config)) {}

  // Http::StreamDecoderFilter
  Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override;

  // Http::StreamEncoderFilter
  Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap& headers, bool) override;

  // Http::StreamFilterBase
  Http::LocalErrorStatus onLocalReply(const Http::StreamFilterBase::LocalReplyData&) override {
    // For strict mode, if an override was attempted and a local reply was sent (e.g., 503),
    // consider this a failed-closed selection.
    if (override_attempted_ && effective_config_ != nullptr && effective_config_->isStrict() &&
        !accounted_) {
      markFailedClosed();
      accounted_ = true;
    }
    return Http::LocalErrorStatus::Continue;
  }

  Http::SessionStatePtr& sessionStateForTest() { return session_state_; }

private:
  void markOverrideAttempted() { override_attempted_ = true; }
  void markRouted() {
    if (override_attempted_ && effective_config_ != nullptr) {
      if (auto stats = effective_config_->stats(); stats.has_value()) {
        stats->routed_.inc();
      }
    }
  }
  void markFailedClosed() {
    if (override_attempted_ && effective_config_ != nullptr) {
      if (auto stats = effective_config_->stats(); stats.has_value()) {
        stats->failed_closed_.inc();
      }
    }
  }
  void markFailedOpen() {
    if (override_attempted_ && effective_config_ != nullptr) {
      if (auto stats = effective_config_->stats(); stats.has_value()) {
        stats->failed_open_.inc();
      }
    }
  }

  Http::SessionStatePtr session_state_;
  StatefulSessionConfigSharedPtr config_;
  bool override_attempted_{false};
  bool accounted_{false};
  // Cached route config to avoid repeated resolution and dynamic_cast issues in tests.

Review comment (bug):
is `if (override_attempted_ && effective_config_ != nullptr)` necessary? I think when we calling these method, all these conditions should always be true.

Code after:
  StatefulSession(StatefulSessionConfigSharedPtr config) : config_(std::move(config)) {}

  // Http::StreamDecoderFilter
  Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override;

  // Http::StreamEncoderFilter
  Http::FilterHeadersStatus encodeHeaders(Http::ResponseHeaderMap& headers, bool) override;

  Http::SessionStatePtr& sessionStateForTest() { return session_state_; }

private:
  void markRouted() {
    if (auto stats = effective_config_->stats(); stats.has_value()) {
      stats->routed_.inc();
    }
  }
  void markFailedClosed() {
    if (auto stats = effective_config_->stats(); stats.has_value()) {
      stats->failed_closed_.inc();
    }
  }
  void markFailedOpen() {
    if (auto stats = effective_config_->stats(); stats.has_value()) {
      stats->failed_open_.inc();
    }
  }

  Http::SessionStatePtr session_state_;
  StatefulSessionConfigSharedPtr config_;
  // Cached effective config resolved from route or base config.
  StatefulSessionConfig* effective_config_{nullptr};
};

} // namespace StatefulSession
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy