fix(matrix,simplex): tolerate malformed HERMES_*_TEXT_BATCH_* delay env vars - #49719
fix(matrix,simplex): tolerate malformed HERMES_*_TEXT_BATCH_* delay env vars#49719ly-wang19 wants to merge 1 commit into
Conversation
|
Related: #48771 (feishu env-guard), #48757 (wecom env-guard) — same |
ea4eb79 to
d047d43
Compare
|
Rebased onto latest |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for carrying the missed Matrix and SimpleX env-float hardening forward. The underlying direct-construction defect is present on current main at plugins/platforms/matrix/adapter.py:926-931 and plugins/platforms/simplex/adapter.py:194-196; utils.env_float provides the intended fallback at utils.py:421-429.
Problems
- The added Matrix comments and test docstring say malformed input aborts gateway boot. Current plugin creation catches factory exceptions in
gateway/platform_registry.py:318-328, andGatewayRunner.start()continues when no adapter is returned atgateway/run.py:7047-7060. Please describe the current consequence as Matrix adapter creation failing instead. - The SimpleX production change has no corresponding malformed-env regression test. Its current adapter-init coverage lives in
tests/gateway/test_simplex_plugin.py:121-142.
Suggested changes
- Correct the stale gateway-boot explanation in the added Matrix comments/tests.
- Add a SimpleX malformed-delay fallback test alongside its adapter-init tests.
Automated hermes-sweeper review.
| self._text_batch_split_delay_seconds = float( | ||
| os.getenv("HERMES_MATRIX_TEXT_BATCH_SPLIT_DELAY_SECONDS", "2.0") | ||
| # env_float tolerates a malformed value (e.g. a "0.6s" unit-suffix typo) | ||
| # instead of raising ValueError out of __init__ and crashing gateway |
There was a problem hiding this comment.
Current main no longer lets this exception abort the full gateway boot: PlatformRegistry.create_adapter() catches factory exceptions at gateway/platform_registry.py:318-328, and GatewayRunner.start() continues when it receives no adapter. Please describe this as failed Matrix adapter creation rather than a gateway-wide crash.
| ) | ||
| # env_float tolerates a malformed value instead of raising out of | ||
| # __init__ (mirrors the other text-batch adapters). | ||
| self._text_batch_delay = env_float("HERMES_SIMPLEX_TEXT_BATCH_DELAY", 0.8) |
There was a problem hiding this comment.
Please add a malformed HERMES_SIMPLEX_TEXT_BATCH_DELAY regression case in tests/gateway/test_simplex_plugin.py alongside the existing adapter-init tests, so this changed fallback path is covered as well as Matrix.
…nv vars a7dd98c swept the unguarded float(os.getenv()) text-batch delay casts onto the safe utils.env_float helper for Feishu/WeCom/Discord/Telegram, but missed the identical sibling sites in the Matrix and SimpleX adapters. A non-numeric value (e.g. a `HERMES_MATRIX_TEXT_BATCH_DELAY_SECONDS=0.6s` unit-suffix typo) raises an uncaught ValueError in MatrixAdapter.__init__, which propagates through GatewayRunner._create_adapter and the unguarded platform-init loop in start(), aborting gateway boot when Matrix is enabled. The same __init__ already guards MATRIX_ROOM_IDENTITY_TTL_SECONDS with try/except, and the sweep's own message says only already-guarded sites were left untouched — so these are overlooked misses, not intentional fail-fast. Route both through env_float (default-on-malformed), matching the four sibling adapters. Adds Matrix init tests; the malformed cases raise without the fix.
|
Rebased onto current Both now go through |
d047d43 to
3bb2028
Compare
Summary
Follow-up to
a7dd98c86("guard remaining malformed int/float env var casts with utils helpers").That sweep routed the
HERMES_<PLATFORM>_TEXT_BATCH_*delay casts through the safeutils.env_floathelper for Feishu, WeCom, Discord, and Telegram — but missed the identical sibling sites in the Matrix and SimpleX adapters, which still do a rawfloat(os.getenv(...)):A non-numeric value — e.g. a realistic unit-suffix typo
HERMES_MATRIX_TEXT_BATCH_DELAY_SECONDS=0.6s— makesfloat("0.6s")raise an uncaughtValueErrorinsideMatrixAdapter.__init__. That propagates throughGatewayRunner._create_adapterand the unguarded platform-init loop inGatewayRunner.start(), so the whole gateway fails to boot when Matrix is enabled.This is an overlooked miss, not an intentional fail-fast:
__init__already guardsMATRIX_ROOM_IDENTITY_TTL_SECONDSwithtry/except+ fallback;Fix
Route both adapters' casts through
env_float(returns the default onValueError/TypeError), exactly as the four siblings were converted. No behavior change for valid values; the empty-string case also degrades to the default instead of raising.Tests
tests/gateway/test_matrix.py::TestMatrixTextBatchEnvParsing— constructsMatrixAdapterwith a malformedHERMES_MATRIX_TEXT_BATCH_DELAY_SECONDS/..._SPLIT_DELAY_SECONDSand asserts init no longer raises and falls back to the default; plus a valid-value case. The two malformed cases fail without the fix (ValueErrorout of__init__).