filters: allow envoy filters to be created as exception free

[alyssawilk]

I tried my best here to keep churn minimal, so the default APIs are the same for almost all cc code (though not for tests) and you opt into exceptionless filters.

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

envoyproxy/envoy-mobile#176

[repokitteh-read-only]

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #30765 was opened by alyssawilk.

see: more, trace.

[alyssawilk]

/retest

[RyanTheOptimist]

Wow, awesome! Would you mind updating the PR description briefly to explain the new class name and factory API that this PR introduces?

[ravenblackx]

I don't understand how this construct is working? EXPECT_TRUE doesn't throw an exception does it? And if the inner part threw an exception then .ok() wouldn't be true so it seems like it ought to fail either way?
Even if it does work, I think the concept would be more readable like

EXPECT_THAT(admission_control_filter_factory
                      .createFilterFactoryFromProtoTyped(proto, "whatever", dual_info_,
                                                         factory_context.getServerFactoryContext()), 
    HasStatusMessage(HasSubstr("Success rate threshold cannot be less than 1.0%.")));

Or, if the thing is that it's throwing an exception and the EXPECT_TRUE is being ignored, then

  EXPECT_THROW_WITH_MESSAGE(
      admission_control_filter_factory
                      .createFilterFactoryFromProtoTyped(proto, "whatever", dual_info_,
                                                         factory_context.getServerFactoryContext())
                      .IgnoreResult(),
    EnvoyException, "Success rate threshold cannot be less than 1.0%.");

[alyssawilk]

the assert is being ignored.
no member named 'IgnoreResult' in 'absl::StatusOr<std::function<void (Envoy::Http::FilterChainFactoryCallbacks &)>>' ditto absl::Status not sure what you're suggesting I use?

[ravenblackx]

Hm, memory failure on this. Just dropping the ASSERT/EXPECT would be what I would expect to see, I assumed they were there because there was an ABSL_MUST_USE_RESULT tag involved.
Bit of a search suggests that's probably right (.ok() is tagged with ABSL_MUST_USE_RESULT), and I meant replace .ok() with .IgnoreError() (from here)

[alyssawilk]

oooh yeah that's much cleaner. on it!

[wbpcode]

LGTM. Thanks.

Seems there are three type of ways to handle the return value when we expect a exception. Call thevalue() directly, call the IgnoreError() directly, call the status().IgnoreError(). Is it oversight or by design? (But it's OK for me if CI is passed. I think we will gradually revise these exception to be error message checking?)

[wbpcode]

nit: status().IgnoreError()?

[wbpcode]

nit: status().IgnoreError()?

[wbpcode]

ditto.