From 765d8520d419b4ac10c490de7feaa516964d27a7 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 20 Apr 2026 15:48:19 -0700 Subject: [PATCH] fix(streaming): quota error detection, error persistence, stream_end session_id fix (#767) - quota_exhausted error type: distinguishes credit exhaustion from rate limits - Streaming errors persisted to session file so they survive page reload - _error flag excludes persisted errors from subsequent LLM API calls - stream_end and title SSE events use original session_id (not s.session_id which rotates during context compaction) Fixes #739, #652, #653 --- CHANGELOG.md | 7 ++ api/streaming.py | 175 +++++++++++++++++++++---------- static/messages.js | 3 +- tests/test_issue739_652_653.py | 124 ++++++++++++++++++++++ tests/test_issues_373_374_375.py | 3 +- tests/test_provider_mismatch.py | 32 +++--- tests/test_sprint41.py | 4 +- 7 files changed, 278 insertions(+), 70 deletions(-) create mode 100644 tests/test_issue739_652_653.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d5e1299..6e34ffae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Hermes Web UI -- Changelog +## [v0.50.116] — 2026-04-20 + +### Fixed +- **Session errors survive page reload** — provider quota exhaustion, rate limit, auth, and agent errors are now persisted to the session file as a special error message. Reloading the page after an error no longer shows a blank conversation. Error messages are excluded from the next API call's conversation history so the LLM never sees its own error as prior context. (#739) +- **Quota/credit exhaustion shows a distinct error** — "Out of credits" now appears instead of the generic "No response received" message when a Codex or other provider account runs out of credits. Both the silent-failure path and the exception path now classify `insufficient_credits` / `quota_exceeded` separately from rate limits, with a targeted hint to top up the balance or switch providers. (#739) +- **Context compaction no longer hangs the session** — when `run_conversation()` rotates the session_id during context compaction, `stream_end` now uses the original session_id (captured before the run), matching what the client captured in `activeSid`. Previously the mismatch caused the EventSource to stay open, trigger a reconnect loop, and show "Connection lost." The same fix also corrects the `title` SSE event. (#652, #653) + ## [v0.50.115] — 2026-04-20 ### Removed diff --git a/api/streaming.py b/api/streaming.py index d8cb5930..94adffdf 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -540,7 +540,7 @@ def _run_background_title_update(session_id: str, user_text: str, assistant_text _put_title_status(put_event, session_id, source, 'local_summary', s.title, raw_preview) else: _put_title_status(put_event, session_id, source, llm_status, s.title, raw_preview) - put_event('title', {'session_id': s.session_id, 'title': s.title}) + put_event('title', {'session_id': session_id, 'title': s.title}) else: _put_title_status(put_event, session_id, 'skipped', source or 'unchanged', current, raw_preview) finally: @@ -578,6 +578,9 @@ def _sanitize_messages_for_api(messages): for msg in messages: if not isinstance(msg, dict): continue + # Skip persisted error markers — never send them to the LLM as prior context. + if msg.get('_error'): + continue role = msg.get('role') if role == 'tool': tid = msg.get('tool_call_id') or '' @@ -1194,31 +1197,66 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta if not _assistant_added and not _token_sent: _last_err = getattr(agent, '_last_error', None) or result.get('error') or '' _err_str = str(_last_err) if _last_err else '' - _is_auth = ( - '401' in _err_str - or (_last_err and 'AuthenticationError' in type(_last_err).__name__) - or 'authentication' in _err_str.lower() - or 'unauthorized' in _err_str.lower() - or 'invalid api key' in _err_str.lower() - or 'invalid_api_key' in _err_str.lower() + _err_lower = _err_str.lower() + _is_quota = ( + 'insufficient credit' in _err_lower + or 'credit balance' in _err_lower + or 'credits exhausted' in _err_lower + or 'quota_exceeded' in _err_lower + or 'quota exceeded' in _err_lower + or 'exceeded your current quota' in _err_lower ) - if _is_auth: - put('apperror', { - 'message': _err_str or 'Authentication failed — check your API key.', - 'type': 'auth_mismatch', - 'hint': ( - 'The selected model may not be supported by your configured provider or ' - 'your API key is invalid. Run `hermes model` in your terminal to ' - 'update credentials, then restart the WebUI.' - ), - }) + _is_auth = ( + not _is_quota and ( + '401' in _err_str + or (_last_err and 'AuthenticationError' in type(_last_err).__name__) + or 'authentication' in _err_lower + or 'unauthorized' in _err_lower + or 'invalid api key' in _err_lower + or 'invalid_api_key' in _err_lower + ) + ) + if _is_quota: + _err_label = 'Out of credits' + _err_type = 'quota_exhausted' + _err_hint = 'Your provider account is out of credits. Top up your balance or switch providers via `hermes model`.' + elif _is_auth: + _err_label = 'Authentication failed' + _err_type = 'auth_mismatch' + _err_hint = ( + 'The selected model may not be supported by your configured provider or ' + 'your API key is invalid. Run `hermes model` in your terminal to ' + 'update credentials, then restart the WebUI.' + ) else: - put('apperror', { - 'message': _err_str or 'The agent returned no response. Check your API key and model selection.', - 'type': 'no_response', - 'hint': 'Verify your API key is valid and the selected model is available for your account.', - }) - return # Don't emit done — the apperror already closes the stream on the client + _err_label = 'No response received' + _err_type = 'no_response' + _err_hint = 'Verify your API key is valid and the selected model is available for your account.' + put('apperror', { + 'message': _err_str or f'{_err_label}.', + 'type': _err_type, + 'hint': _err_hint, + }) + # Clear stream/pending state so the session does not appear + # "agent_running" on reload after a silent failure. + s.active_stream_id = None + s.pending_user_message = None + s.pending_attachments = [] + s.pending_started_at = None + # Persist the error so it survives page reload. + # _error=True ensures _sanitize_messages_for_api excludes it from + # subsequent API calls so the LLM never sees its own error as prior context. + s.messages.append({ + 'role': 'assistant', + 'content': f'**{_err_label}:** {_err_str or _err_label}\n\n*{_err_hint}*', + 'timestamp': int(time.time()), + '_error': True, + }) + try: + s.save() + except Exception: + pass + return # apperror already closes the stream on the client side # ── Handle context compression side effects ── # If compression fired inside run_conversation, the agent may have @@ -1346,7 +1384,10 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta daemon=True, ).start() else: - put('stream_end', {'session_id': s.session_id}) + # Use the original session_id parameter (never reassigned), not s.session_id + # which may be rotated during context compression. The client captured + # activeSid = original session_id so they must match for stream_end to close. + put('stream_end', {'session_id': session_id}) finally: # Unregister the gateway approval callback and unblock any threads # still waiting on approval (e.g. stream cancelled mid-approval). @@ -1372,44 +1413,70 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta except Exception as e: print('[webui] stream error:\n' + traceback.format_exc(), flush=True) + err_str = str(e) + _exc_lower = err_str.lower() + # Classify before saving so the error message can be persisted to the session. + # Check quota exhaustion first — OpenAI billing 429s use insufficient_quota which + # also matches rate-limit patterns, so order matters. + _exc_is_quota = ( + 'insufficient credit' in _exc_lower + or 'credit balance' in _exc_lower + or 'credits exhausted' in _exc_lower + or 'quota_exceeded' in _exc_lower + or 'quota exceeded' in _exc_lower + or 'exceeded your current quota' in _exc_lower + ) + _exc_is_rate_limit = (not _exc_is_quota) and ( + 'rate limit' in _exc_lower or '429' in err_str or 'RateLimitError' in type(e).__name__ + ) + _exc_is_auth = ( + '401' in err_str + or 'AuthenticationError' in type(e).__name__ + or 'authentication' in _exc_lower + or 'unauthorized' in _exc_lower + or 'invalid api key' in _exc_lower + or 'no cookie auth credentials' in _exc_lower + ) + if _exc_is_quota: + _exc_label, _exc_type, _exc_hint = ( + 'Out of credits', 'quota_exhausted', + 'Your provider account is out of credits. Top up your balance or switch providers via `hermes model`.', + ) + elif _exc_is_rate_limit: + _exc_label, _exc_type, _exc_hint = ( + 'Rate limit reached', 'rate_limit', + 'Rate limit reached. The fallback model (if configured) was also exhausted. Try again in a moment.', + ) + elif _exc_is_auth: + _exc_label, _exc_type, _exc_hint = ( + 'Authentication error', 'auth_mismatch', + 'The selected model may not be supported by your configured provider. ' + 'Run `hermes model` in your terminal to switch providers, then restart the WebUI.', + ) + else: + _exc_label, _exc_type, _exc_hint = 'Error', 'error', '' if s is not None: s.active_stream_id = None s.pending_user_message = None s.pending_attachments = [] s.pending_started_at = None + # Persist the error so it survives page reload. + # _error=True ensures _sanitize_messages_for_api excludes it from subsequent + # API calls so the LLM never sees its own error as prior context on the next turn. + s.messages.append({ + 'role': 'assistant', + 'content': f'**{_exc_label}:** {err_str}' + (f'\n\n*{_exc_hint}*' if _exc_hint else ''), + 'timestamp': int(time.time()), + '_error': True, + }) try: s.save() except Exception: pass - err_str = str(e) - # Detect rate limit errors specifically so the client can show a helpful card - # rather than the generic "Connection lost" message - is_rate_limit = 'rate limit' in err_str.lower() or '429' in err_str or 'RateLimitError' in type(e).__name__ - is_auth_error = ( - '401' in err_str - or 'AuthenticationError' in type(e).__name__ - or 'authentication' in err_str.lower() - or 'unauthorized' in err_str.lower() - or 'invalid api key' in err_str.lower() - or 'no cookie auth credentials' in err_str.lower() - ) - if is_rate_limit: - put('apperror', { - 'message': err_str, - 'type': 'rate_limit', - 'hint': 'Rate limit reached. The fallback model (if configured) was also exhausted. Try again in a moment.', - }) - elif is_auth_error: - put('apperror', { - 'message': err_str, - 'type': 'auth_mismatch', - 'hint': ( - 'The selected model may not be supported by your configured provider. ' - 'Run `hermes model` in your terminal to switch providers, then restart the WebUI.' - ), - }) - else: - put('apperror', {'message': err_str, 'type': 'error'}) + _apperror_payload: dict = {'message': err_str, 'type': _exc_type} + if _exc_hint: + _apperror_payload['hint'] = _exc_hint + put('apperror', _apperror_payload) finally: _clear_thread_env() # TD1: always clear thread-local context with STREAMS_LOCK: diff --git a/static/messages.js b/static/messages.js index 4475d8e4..38818fd9 100644 --- a/static/messages.js +++ b/static/messages.js @@ -520,9 +520,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ try{ const d=JSON.parse(e.data); const isRateLimit=d.type==='rate_limit'; + const isQuotaExhausted=d.type==='quota_exhausted'; const isAuthMismatch=d.type==='auth_mismatch'; const isNoResponse=d.type==='no_response'; - const label=isRateLimit?'Rate limit reached':isAuthMismatch?(typeof t==='function'?t('provider_mismatch_label'):'Provider mismatch'):isNoResponse?'No response received':'Error'; + const label=isQuotaExhausted?'Out of credits':isRateLimit?'Rate limit reached':isAuthMismatch?(typeof t==='function'?t('provider_mismatch_label'):'Provider mismatch'):isNoResponse?'No response received':'Error'; const hint=d.hint?`\n\n*${d.hint}*`:''; S.messages.push({role:'assistant',content:`**${label}:** ${d.message}${hint}`}); }catch(_){ diff --git a/tests/test_issue739_652_653.py b/tests/test_issue739_652_653.py new file mode 100644 index 00000000..254649f7 --- /dev/null +++ b/tests/test_issue739_652_653.py @@ -0,0 +1,124 @@ +""" +Tests for streaming error handling fixes: + #739 — quota/credit exhaustion detected as distinct error type + persisted to session + #652 — context compaction session_id rotation: stream_end uses original session_id + #653 — bad tool call hang: same stream_end fix applies + +All static tests (no live server required). +""" +import ast +import re +import pathlib + +STREAMING = pathlib.Path(__file__).parent.parent / 'api' / 'streaming.py' +MESSAGES_JS = pathlib.Path(__file__).parent.parent / 'static' / 'messages.js' + +streaming_src = STREAMING.read_text(encoding='utf-8') +messages_js_src = MESSAGES_JS.read_text(encoding='utf-8') + + +# ── #739: Quota exhaustion detection ───────────────────────────────────────── + +class TestQuotaDetection: + """Quota-exhausted errors must be classified separately from rate limits.""" + + def test_quota_patterns_present_in_silent_failure_path(self): + """The silent-failure path checks for credit/quota strings.""" + block = streaming_src + assert 'insufficient credit' in block + assert 'credit balance' in block + assert 'credits exhausted' in block + assert 'quota_exceeded' in block + assert 'exceeded your current quota' in block + + def test_quota_type_emitted_as_quota_exhausted(self): + """The apperror type is 'quota_exhausted', not 'error' or 'rate_limit'.""" + assert "'quota_exhausted'" in streaming_src or '"quota_exhausted"' in streaming_src + + def test_quota_checked_before_rate_limit(self): + """Quota check must appear before the rate-limit check in the exception path. + OpenAI billing 429s overlap with rate-limit patterns.""" + quota_pos = streaming_src.find('_exc_is_quota') + rate_pos = streaming_src.find('_exc_is_rate_limit') + assert quota_pos != -1, '_exc_is_quota not found in exception path' + assert rate_pos != -1, '_exc_is_rate_limit not found in exception path' + assert quota_pos < rate_pos, 'Quota check must appear before rate-limit check' + + def test_rate_limit_excludes_quota(self): + """Rate-limit detection must be guarded so quota errors don't also match.""" + # The pattern: _exc_is_rate_limit = (not _exc_is_quota) and (...) + assert '(not _exc_is_quota)' in streaming_src + + def test_js_quota_label_present(self): + """messages.js renders a 'quota_exhausted' apperror with a distinct label.""" + assert "quota_exhausted" in messages_js_src + assert "Out of credits" in messages_js_src + + +# ── #739: Error persistence across reload ───────────────────────────────────── + +class TestErrorPersistence: + """Errors must be saved to the session so they survive page reload.""" + + def test_silent_failure_appends_error_message(self): + """Silent-failure path appends an _error-marked message before returning.""" + # Must append to s.messages with _error key + assert "s.messages.append(" in streaming_src + assert "'_error': True" in streaming_src + + def test_silent_failure_calls_save_before_return(self): + """save() must be called after appending the error message.""" + # Find the silent failure block area and verify save precedes return + pattern = re.compile( + r"s\.messages\.append\(.*?'_error': True.*?\).*?s\.save\(\).*?return", + re.DOTALL + ) + assert pattern.search(streaming_src), \ + "save() must be called after appending the error message in the silent-failure path" + + def test_exception_path_appends_error_message(self): + """Exception path also persists the error to the session.""" + # Both paths should have _error persistence + count = streaming_src.count("'_error': True") + assert count >= 2, f"Expected at least 2 _error persistence sites, found {count}" + + def test_sanitize_skips_error_messages(self): + """_sanitize_messages_for_api must not send _error messages to the LLM.""" + assert "msg.get('_error')" in streaming_src or 'msg.get("_error")' in streaming_src + # The skip must come before the role/tool filtering logic + error_skip_pos = streaming_src.find("msg.get('_error')") + tool_filter_pos = streaming_src.find("if role == 'tool':") + assert error_skip_pos < tool_filter_pos, \ + "_error skip must appear before the tool-role filter in _sanitize_messages_for_api" + + +# ── #652/#653: Context compaction stream_end fix ────────────────────────────── + +class TestStreamEndSessionId: + """stream_end must use the original session_id param, not s.session_id.""" + + def test_non_bg_title_stream_end_uses_session_id_param(self): + """When no background title is spawned, stream_end should use original session_id.""" + # The fixed code: put('stream_end', {'session_id': session_id}) + # Not: put('stream_end', {'session_id': s.session_id}) + # Verify the pattern appears in the non-background-title branch + assert "put('stream_end', {'session_id': session_id})" in streaming_src + + def test_background_title_thread_stream_end_uses_session_id_param(self): + """Background title thread also emits stream_end with original session_id.""" + # In _run_background_title_update: put_event('stream_end', {'session_id': session_id}) + # The session_id param is passed from the caller with the original value + assert "put_event('stream_end', {'session_id': session_id})" in streaming_src + + def test_s_session_id_not_used_in_stream_end(self): + """s.session_id (which may be rotated after compaction) must not appear in stream_end.""" + # Find all stream_end emissions and verify none use s.session_id + for match in re.finditer(r"put[_a-z]*\('stream_end',[^)]+\)", streaming_src): + assert 's.session_id' not in match.group(), \ + f"stream_end uses s.session_id (may be rotated): {match.group()}" + + def test_title_event_uses_original_session_id(self): + """title event in background title thread uses original session_id, not s.session_id.""" + # Client guard: if((d.session_id||activeSid)!==activeSid) return; + # So title must be emitted with the original id + assert "put_event('title', {'session_id': session_id," in streaming_src diff --git a/tests/test_issues_373_374_375.py b/tests/test_issues_373_374_375.py index 93d71728..1d7b9b93 100644 --- a/tests/test_issues_373_374_375.py +++ b/tests/test_issues_373_374_375.py @@ -37,7 +37,8 @@ class TestSilentErrorDetection: """streaming.py must return after emitting apperror (not also emit done).""" # The return statement must come after the put('apperror') for no_response no_resp_pos = STREAMING_PY.find("'no_response'") - return_pos = STREAMING_PY.find("return # Don't emit done", no_resp_pos) + # Comment updated: "apperror already closes the stream on the client side" + return_pos = STREAMING_PY.find("return # apperror already closes the stream", no_resp_pos) assert no_resp_pos != -1, "no_response type not found in streaming.py" assert return_pos != -1, ( "streaming.py must return after emitting apperror to prevent also emitting done (#373)" diff --git a/tests/test_provider_mismatch.py b/tests/test_provider_mismatch.py index 93771785..14c2a8de 100644 --- a/tests/test_provider_mismatch.py +++ b/tests/test_provider_mismatch.py @@ -37,30 +37,33 @@ class TestStreamingAuthErrorDetection: ) def test_is_auth_error_flag_defined(self): - """is_auth_error variable must exist in the error handler.""" + """auth error variable must exist in the error handler (exception path and silent-failure path).""" src = _read("api/streaming.py") - assert "is_auth_error" in src, ( - "is_auth_error flag not found in streaming.py" + # Variable renamed to _exc_is_auth in exception path, _is_auth in silent-failure path + assert "_exc_is_auth" in src or "_is_auth" in src, ( + "auth error flag not found in streaming.py" ) def test_auth_error_detects_401(self): """'401' must be part of the auth error detection logic.""" src = _read("api/streaming.py") # Find the is_auth_error block - idx = src.find("is_auth_error") + # Variable renamed to _exc_is_auth in exception path, _is_auth in silent-failure path + idx = src.find("_exc_is_auth") assert idx != -1 - block = src[idx:idx + 400] + block = src[idx:idx + 500] assert "'401'" in block or '"401"' in block, ( - "'401' not in is_auth_error detection block" + "'401' not in auth error detection block" ) def test_auth_error_detects_unauthorized(self): """'unauthorized' must be part of the auth error detection logic.""" src = _read("api/streaming.py") - idx = src.find("is_auth_error") - block = src[idx:idx + 400] + # Variable renamed to _exc_is_auth in exception path + idx = src.find("_exc_is_auth") + block = src[idx:idx + 500] assert "unauthorized" in block.lower(), ( - "'unauthorized' not in is_auth_error detection block" + "'unauthorized' not in auth error detection block" ) def test_auth_error_hint_mentions_hermes_model(self): @@ -77,11 +80,14 @@ class TestStreamingAuthErrorDetection: def test_auth_error_does_not_catch_rate_limit(self): """Rate limit errors must not be reclassified as auth_mismatch.""" src = _read("api/streaming.py") - # is_rate_limit must come before is_auth_error in the elif chain - rl_idx = src.find("is_rate_limit") - ae_idx = src.find("is_auth_error") + # Variables renamed: _exc_is_rate_limit / _exc_is_auth in exception path + # Quota check comes first (before rate limit), then rate limit, then auth + rl_idx = src.find("_exc_is_rate_limit") + ae_idx = src.find("_exc_is_auth") + assert rl_idx != -1, "_exc_is_rate_limit not found in streaming.py exception path" + assert ae_idx != -1, "_exc_is_auth not found in streaming.py exception path" assert rl_idx < ae_idx, ( - "is_rate_limit check should precede is_auth_error — " + "_exc_is_rate_limit check should precede _exc_is_auth — " "rate limit errors must not be mistaken for auth errors" ) diff --git a/tests/test_sprint41.py b/tests/test_sprint41.py index 7ca41067..48708ccf 100644 --- a/tests/test_sprint41.py +++ b/tests/test_sprint41.py @@ -156,8 +156,10 @@ class TestIssue495TitleStreaming(unittest.TestCase): ) def test_streaming_emits_title_sse_event(self): + # After the stream_end fix, title uses original session_id param (not s.session_id + # which can be rotated during context compression — see #652 fix) self.assertIn( - "put_event('title', {'session_id': s.session_id, 'title': s.title})", + "put_event('title', {'session_id': session_id, 'title': s.title})", STREAMING_PY, "streaming.py should emit a title SSE event when title is updated", )