Skip to content

http: migrate ext proc/... to exception free#46042

Merged
wbpcode merged 3 commits into
envoyproxy:mainfrom
wbpcode:dev-make-http-exception-free-10
Jul 9, 2026
Merged

http: migrate ext proc/... to exception free#46042
wbpcode merged 3 commits into
envoyproxy:mainfrom
wbpcode:dev-make-http-exception-free-10

Conversation

@wbpcode

@wbpcode wbpcode commented Jul 8, 2026

Copy link
Copy Markdown
Member

Commit Message: http: migrate ext proc/... to exception free
Additional Description:
Migrate exceptions of these filters to absl::Status and absl::StatusOr.

NOTE: This is AI assisted. But I reviewed all the changes and tests, and also did some manually update.

Risk Level: low.
Testing: unit/integration.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates parts of the ext_proc HTTP filter (and related request-modifier utilities) toward exception-free construction by propagating configuration/creation failures via absl::Status instead of throwing, and updates tests accordingly.

Changes:

  • Add absl::Status& out-parameters to ext_proc config/expression-related constructors and propagate errors via RETURN_IF_NOT_OK(_REF) patterns.
  • Update ext_proc unit tests/fuzz test to pass and assert/handle creation statuses; modernize some status assertions via StatusHelpers::HasStatus.
  • Simplify a couple of other filter factories to return StatusOr results directly instead of throwing wrappers.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tools/code_format/config.yaml Adds the mapped-attribute builder factory source file to formatting paths.
test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz.cc Passes a creation absl::Status into FilterConfig and returns early on failure.
test/extensions/filters/http/ext_proc/ordering_test.cc Updates FilterConfig construction to use creation_status and asserts success.
test/extensions/filters/http/ext_proc/matching_utils_test.cc Updates ExpressionManager construction in CEL-enabled test paths to use creation_status.
test/extensions/filters/http/ext_proc/mapped_attribute_builder_test.cc Updates MappedAttributeBuilder construction to use creation_status.
test/extensions/filters/http/ext_proc/filter_test.cc Updates FilterConfig construction to use creation_status and asserts success.
test/extensions/filters/http/ext_proc/filter_test_common.cc Updates shared test initialization to use creation_status and asserts success.
test/extensions/filters/http/ext_proc/config_test.cc Switches several failure expectations to HasStatus(...) and avoids exception-based checks.
test/extensions/filters/http/ext_proc/BUILD Adds dependency on status_utility_lib for new status matchers.
test/extensions/filters/http/credential_injector/config_test.cc Updates a server-context test to validate error via returned status message rather than exception.
source/extensions/http/ext_proc/processing_request_modifiers/mapped_attribute_builder/mapped_attribute_builder.h Adds absl::Status& creation_status to MappedAttributeBuilder constructor signature.
source/extensions/http/ext_proc/processing_request_modifiers/mapped_attribute_builder/mapped_attribute_builder.cc Plumbs creation_status through to ExpressionManager.
source/extensions/http/ext_proc/processing_request_modifiers/mapped_attribute_builder/mapped_attribute_builder_factory.cc Constructs MappedAttributeBuilder with out-status and currently throws on failure.
source/extensions/filters/http/header_mutation/config.cc Uses RETURN_IF_NOT_OK_REF instead of manual exception throwing on config creation failures.
source/extensions/filters/http/ext_proc/matching_utils.h Updates ExpressionManager ctor and initExpressions to accept absl::Status&.
source/extensions/filters/http/ext_proc/matching_utils.cc Replaces exception throws with creation_status updates for parse/compile failures.
source/extensions/filters/http/ext_proc/ext_proc.h Updates FilterConfig constructor signature to accept absl::Status&.
source/extensions/filters/http/ext_proc/ext_proc.cc Plumbs creation_status into ExpressionManager construction.
source/extensions/filters/http/ext_proc/config.cc Validates config and returns status instead of throwing; propagates FilterConfig creation status.
source/extensions/filters/http/dynamic_modules/factory.cc Returns the StatusOr from createFilterFactory(...) directly instead of throwing on error.
source/extensions/filters/http/credential_injector/config.cc Returns the StatusOr from helper directly instead of throwing wrapper exceptions.

Comment thread source/extensions/filters/http/ext_proc/matching_utils.h
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
@yanjunxiang-google

Copy link
Copy Markdown
Contributor

/assign @yanjunxiang-google

@yanjunxiang-google

Copy link
Copy Markdown
Contributor

LGTM

@wbpcode
wbpcode requested a review from Copilot July 9, 2026 14:34
@wbpcode
wbpcode enabled auto-merge (squash) July 9, 2026 14:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Comment on lines +39 to +41
creation_status = absl::InvalidArgumentError(
absl::StrCat("failed to create an expression: ", compiled_expression.status().message()));
return expressions;
@wbpcode
wbpcode merged commit 2e51db0 into envoyproxy:main Jul 9, 2026
26 of 28 checks passed
eric846 pushed a commit to envoyproxy/nighthawk that referenced this pull request Jul 13, 2026
- Update the ENVOY_COMMIT and ENVOY_SHA in bazel/repositories.bzl to the latest Envoy's commit.
- Update tools/code_format/config.yaml to envoyproxy/envoy#46042

Signed-off-by: fei-deng <101672975+fei-deng@users.noreply.github.com>
yanavlasov pushed a commit that referenced this pull request Jul 17, 2026
The zookeeper_proxy network filter was one of the few remaining network
filters whose config factory still threw `EnvoyException` instead of
returning `absl::StatusOr`, which kept its whole directory on the
`tools/code_format/config.yaml` exception allowlist and outside the
no-throw check. This change switches `ZooKeeperConfigFactory` from
`Common::FactoryBase` to `Common::ExceptionFreeFactoryBase` so
`createFilterFactoryFromProtoTyped` returns
`absl::StatusOr<Network::FilterFactoryCb>`, converts the two config-time
throws (the duplicated-opcode check and the unknown-opcode lookup) to
`absl::InvalidArgumentError` with byte-identical message text, and
removes the zookeeper_proxy entry from the exception allowlist so
check_format now enforces no-throw for the entire directory. The
ZooKeeper filter's data plane was already made exception free in #31485;
these two config-time throws were all that remained, so this finishes
zookeeper_proxy specifically. It continues the exception-free migration
tracked in #27412, following the same pattern as recently merged
network-filter (#46111) and HTTP-filter (#46042) work.

**Commit Message:** zookeeper_proxy: migrate config validation to
exception free

**Additional Description:** `parseLatencyThresholdOverrides` becomes a
public static helper returning
`absl::StatusOr<LatencyThresholdOverrideMap>`; the factory parses before
constructing `ZooKeeperFilterConfig`, whose constructor now takes the
parsed map and can no longer fail.

**Risk Level:** Low. Config-load-time only refactor with no data plane
change; error messages and failure semantics are unchanged
(`EnvoyException` becomes `absl::InvalidArgumentError` with identical
text, surfaced through the same config rejection path).

**Testing:**
- `//test/extensions/filters/network/zookeeper_proxy:config_test` and
`:filter_test` pass in the CI clang docker image. The duplicated-opcode
case was converted from `EXPECT_THROW_WITH_REGEX` to a status matcher
with the identical message substring; a new
`UnknownOpcodeInLatencyThresholdOverrides` test calls the now-public
static helper directly with an out-of-range opcode and asserts
`InvalidArgumentError` with the identical text, covering the error
branch so per-directory coverage does not regress. filter_test's
existing tests run unchanged through the updated `initialize()` helper.
- Countable metric: throw sites in the directory go from 2 to 0 (`grep
-rE 'throw|THROW'`), and the allowlist entry is removed. Mutation proof:
with the allowlist entry removed, injecting a `throw` into filter.cc
makes `check_format` fail with "Don't introduce throws into
exception-free files"; at the parent commit the same injected throw
passes, confirming the directory is now enforced.

**Docs Changes:** N/A

**Release Notes:** N/A (internal refactor, no behavior change; matches
the no-changelog precedent of #46111 and #46042)

**Platform Specific Features:** N/A

Part of #27412

**Generative AI disclosure:** Developed with AI assistance (Claude
Code); I reviewed and understand every line and take full ownership of
the submission.

Signed-off-by: David Vo <davidvo@lyft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants