From d9a4983cfc2c4ccde28c64bd9b64a7eb7103ccef Mon Sep 17 00:00:00 2001 From: rnetser Date: Thu, 22 Jan 2026 16:24:34 +0200 Subject: [PATCH 1/9] fix(github-api): handle authentication errors in add_api_users_to_auto_verified_and_merged_users Previously, if a GitHub API token failed authentication with a 401 error, the GithubException was not caught, causing the server to crash. Added try/except block to catch GithubException and log a warning instead of crashing, allowing the server to continue with other valid tokens. --- webhook_server/libs/github_api.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index b24cdd1b..2481906b 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -595,14 +595,24 @@ async def process(self) -> Any: def add_api_users_to_auto_verified_and_merged_users(self) -> None: apis_and_tokens = get_apis_and_tokes_from_config(config=self.config) - for _api, _ in apis_and_tokens: + for _api, _token in apis_and_tokens: + token_suffix = f"...{_token[-4:]}" if _token else "unknown" if _api.rate_limiting[-1] == 60: self.logger.warning( - f"{self.log_prefix} API has rate limit set to 60 which indicates an invalid token, skipping" + f"{self.log_prefix} API has rate limit set to 60 which indicates an invalid token " + f"(token ending in '{token_suffix}'), skipping" ) continue - self.auto_verified_and_merged_users.append(_api.get_user().login) + try: + _api_user = _api.get_user().login + except GithubException as ex: + self.logger.warning( + f"{self.log_prefix} Failed to get API user for token ending in '{token_suffix}', skipping. {ex}" + ) + continue + + self.auto_verified_and_merged_users.append(_api_user) def prepare_log_prefix(self, pull_request: PullRequest | None = None) -> str: return prepare_log_prefix( From 2909797a07fafb2e72927b39a79ca1f11070851d Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 28 Jan 2026 08:12:42 +0200 Subject: [PATCH 2/9] update log level --- webhook_server/libs/github_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 2481906b..1287ca46 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -598,7 +598,7 @@ def add_api_users_to_auto_verified_and_merged_users(self) -> None: for _api, _token in apis_and_tokens: token_suffix = f"...{_token[-4:]}" if _token else "unknown" if _api.rate_limiting[-1] == 60: - self.logger.warning( + self.logger.error( f"{self.log_prefix} API has rate limit set to 60 which indicates an invalid token " f"(token ending in '{token_suffix}'), skipping" ) @@ -607,7 +607,7 @@ def add_api_users_to_auto_verified_and_merged_users(self) -> None: try: _api_user = _api.get_user().login except GithubException as ex: - self.logger.warning( + self.logger.error( f"{self.log_prefix} Failed to get API user for token ending in '{token_suffix}', skipping. {ex}" ) continue From 6cfb5168b36e3a2dfc98373910d8255d8cac069a Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 28 Jan 2026 19:39:29 +0200 Subject: [PATCH 3/9] update log level to exception --- webhook_server/libs/github_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 1287ca46..2c11ce1b 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -598,7 +598,7 @@ def add_api_users_to_auto_verified_and_merged_users(self) -> None: for _api, _token in apis_and_tokens: token_suffix = f"...{_token[-4:]}" if _token else "unknown" if _api.rate_limiting[-1] == 60: - self.logger.error( + self.logger.exception( f"{self.log_prefix} API has rate limit set to 60 which indicates an invalid token " f"(token ending in '{token_suffix}'), skipping" ) @@ -607,7 +607,7 @@ def add_api_users_to_auto_verified_and_merged_users(self) -> None: try: _api_user = _api.get_user().login except GithubException as ex: - self.logger.error( + self.logger.exception( f"{self.log_prefix} Failed to get API user for token ending in '{token_suffix}', skipping. {ex}" ) continue From f9136c01e7b928d4e608a2356dc0fd0e1ea397a6 Mon Sep 17 00:00:00 2001 From: rnetser Date: Wed, 28 Jan 2026 21:51:43 +0200 Subject: [PATCH 4/9] fix(github-api): add try/except for rate_limiting access Address CodeRabbit review comment: wrap _api.rate_limiting[-1] access in try/except to handle GithubException on invalid tokens. Also fix logger.exception to logger.warning for non-exception context. --- webhook_server/libs/github_api.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 2c11ce1b..df0f25eb 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -597,8 +597,17 @@ def add_api_users_to_auto_verified_and_merged_users(self) -> None: apis_and_tokens = get_apis_and_tokes_from_config(config=self.config) for _api, _token in apis_and_tokens: token_suffix = f"...{_token[-4:]}" if _token else "unknown" - if _api.rate_limiting[-1] == 60: - self.logger.exception( + try: + rate_limit_remaining = _api.rate_limiting[-1] + except GithubException as ex: + self.logger.warning( + f"{self.log_prefix} Failed to get API rate limit for token ending in '{token_suffix}', " + f"skipping. {ex}" + ) + continue + + if rate_limit_remaining == 60: + self.logger.warning( f"{self.log_prefix} API has rate limit set to 60 which indicates an invalid token " f"(token ending in '{token_suffix}'), skipping" ) From dccc59b1be685bd33c27b3851dd2ae7a264434a1 Mon Sep 17 00:00:00 2001 From: rnetser Date: Thu, 29 Jan 2026 12:23:20 +0200 Subject: [PATCH 5/9] fix(github-api): make add_api_users async with asyncio.to_thread Address CodeRabbit review: convert blocking PyGithub calls (_api.rate_limiting and _api.get_user()) to async using asyncio.to_thread() to prevent blocking the event loop. - Convert add_api_users_to_auto_verified_and_merged_users to async - Move call from __init__ to process() (async context) - Wrap blocking PyGithub calls with asyncio.to_thread() - Update test to handle async method --- webhook_server/libs/github_api.py | 13 ++++++++----- webhook_server/tests/test_github_api.py | 7 ++++--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index df0f25eb..6880548f 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -178,8 +178,8 @@ def __init__(self, hook_data: dict[Any, Any], headers: Headers, logger: logging. # This prevents predictable paths and ensures isolation between concurrent webhook handlers self.clone_repo_dir: str = tempfile.mkdtemp(prefix=f"github-webhook-{self.repository_name}-") self._repo_cloned: bool = False # Track if repository has been cloned - # Initialize auto-verified users from API users - self.add_api_users_to_auto_verified_and_merged_users() + # Note: auto-verified users from API users are initialized in process() + # because the method is async and requires asyncio.to_thread() for blocking calls self.current_pull_request_supported_retest = self._current_pull_request_supported_retest self.issue_url_for_welcome_msg: str = ( @@ -408,6 +408,9 @@ def redact_output(value: str) -> str: raise RuntimeError(f"Repository clone failed: {ex}") from ex async def process(self) -> Any: + # Initialize auto-verified users from API users (async operation) + await self.add_api_users_to_auto_verified_and_merged_users() + event_log: str = f"Event type: {self.github_event}. event ID: {self.x_github_delivery}" # Start webhook routing context step @@ -593,12 +596,12 @@ async def process(self) -> Any: await self._update_context_metrics() return None - def add_api_users_to_auto_verified_and_merged_users(self) -> None: + async def add_api_users_to_auto_verified_and_merged_users(self) -> None: apis_and_tokens = get_apis_and_tokes_from_config(config=self.config) for _api, _token in apis_and_tokens: token_suffix = f"...{_token[-4:]}" if _token else "unknown" try: - rate_limit_remaining = _api.rate_limiting[-1] + rate_limit_remaining = await asyncio.to_thread(lambda api: api.rate_limiting[-1], _api) except GithubException as ex: self.logger.warning( f"{self.log_prefix} Failed to get API rate limit for token ending in '{token_suffix}', " @@ -614,7 +617,7 @@ def add_api_users_to_auto_verified_and_merged_users(self) -> None: continue try: - _api_user = _api.get_user().login + _api_user = await asyncio.to_thread(lambda api: api.get_user().login, _api) except GithubException as ex: self.logger.exception( f"{self.log_prefix} Failed to get API user for token ending in '{token_suffix}', skipping. {ex}" diff --git a/webhook_server/tests/test_github_api.py b/webhook_server/tests/test_github_api.py index 5e0d58a9..7938655c 100644 --- a/webhook_server/tests/test_github_api.py +++ b/webhook_server/tests/test_github_api.py @@ -621,13 +621,14 @@ def test_init_failed_repository_objects( assert gh.repository is None assert not hasattr(gh, "repository_by_github_app") + @pytest.mark.asyncio @patch("webhook_server.libs.github_api.Config") @patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") @patch("webhook_server.libs.github_api.get_github_repo_api") @patch("webhook_server.libs.github_api.get_repository_github_app_api") @patch("webhook_server.utils.helpers.get_repository_color_for_log_prefix") @patch("webhook_server.libs.github_api.get_apis_and_tokes_from_config") - def test_add_api_users_to_auto_verified_and_merged_users( + async def test_add_api_users_to_auto_verified_and_merged_users( self, mock_get_apis, mock_color, @@ -639,7 +640,7 @@ def test_add_api_users_to_auto_verified_and_merged_users( minimal_headers, logger, ) -> None: - """Test the add_api_users_to_auto_verified_and_merged_users property.""" + """Test the add_api_users_to_auto_verified_and_merged_users method.""" mock_config.return_value.repository = True mock_config.return_value.repository_local_data.return_value = {} mock_get_api.return_value = (Mock(), "token", "apiuser") @@ -689,7 +690,7 @@ def get_value_side_effect(value, *args, **kwargs): mock_api.get_user.return_value = mock_user mock_get_apis.return_value = [(mock_api, "token")] gh = GithubWebhook(minimal_hook_data, minimal_headers, logger) - _ = gh.add_api_users_to_auto_verified_and_merged_users + await gh.add_api_users_to_auto_verified_and_merged_users() assert "test-user" in gh.auto_verified_and_merged_users @patch("webhook_server.libs.github_api.get_apis_and_tokes_from_config") From fc690bb71d91b2a2257947f8b7b22e90d4b3f2b1 Mon Sep 17 00:00:00 2001 From: rnetser Date: Thu, 29 Jan 2026 13:55:35 +0200 Subject: [PATCH 6/9] refactor(github-api): parallelize token checks with asyncio.gather Address CodeRabbit review comments: - Refactor add_api_users_to_auto_verified_and_merged_users to process all tokens concurrently using asyncio.gather instead of sequential loop - Use TEST_GITHUB_TOKEN constant in test for consistency --- webhook_server/libs/github_api.py | 21 +++++++++++++-------- webhook_server/tests/test_github_api.py | 2 +- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 6880548f..e081758c 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -598,33 +598,38 @@ async def process(self) -> Any: async def add_api_users_to_auto_verified_and_merged_users(self) -> None: apis_and_tokens = get_apis_and_tokes_from_config(config=self.config) - for _api, _token in apis_and_tokens: - token_suffix = f"...{_token[-4:]}" if _token else "unknown" + + async def check_token(api: github.Github, token: str) -> str | None: + """Check a single API token and return the user login if valid, None otherwise.""" + token_suffix = f"...{token[-4:]}" if token else "unknown" try: - rate_limit_remaining = await asyncio.to_thread(lambda api: api.rate_limiting[-1], _api) + rate_limit_remaining = await asyncio.to_thread(lambda: api.rate_limiting[-1]) except GithubException as ex: self.logger.warning( f"{self.log_prefix} Failed to get API rate limit for token ending in '{token_suffix}', " f"skipping. {ex}" ) - continue + return None if rate_limit_remaining == 60: self.logger.warning( f"{self.log_prefix} API has rate limit set to 60 which indicates an invalid token " f"(token ending in '{token_suffix}'), skipping" ) - continue + return None try: - _api_user = await asyncio.to_thread(lambda api: api.get_user().login, _api) + _api_user = await asyncio.to_thread(lambda: api.get_user().login) except GithubException as ex: self.logger.exception( f"{self.log_prefix} Failed to get API user for token ending in '{token_suffix}', skipping. {ex}" ) - continue + return None + + return _api_user - self.auto_verified_and_merged_users.append(_api_user) + results = await asyncio.gather(*[check_token(api, token) for api, token in apis_and_tokens]) + self.auto_verified_and_merged_users.extend(user for user in results if user is not None) def prepare_log_prefix(self, pull_request: PullRequest | None = None) -> str: return prepare_log_prefix( diff --git a/webhook_server/tests/test_github_api.py b/webhook_server/tests/test_github_api.py index 7938655c..3f8655a2 100644 --- a/webhook_server/tests/test_github_api.py +++ b/webhook_server/tests/test_github_api.py @@ -688,7 +688,7 @@ def get_value_side_effect(value, *args, **kwargs): mock_user = Mock() mock_user.login = "test-user" mock_api.get_user.return_value = mock_user - mock_get_apis.return_value = [(mock_api, "token")] + mock_get_apis.return_value = [(mock_api, TEST_GITHUB_TOKEN)] gh = GithubWebhook(minimal_hook_data, minimal_headers, logger) await gh.add_api_users_to_auto_verified_and_merged_users() assert "test-user" in gh.auto_verified_and_merged_users From 1ea9802a23834fe02f0258dd2cde90210d98cd23 Mon Sep 17 00:00:00 2001 From: rnetser Date: Mon, 16 Feb 2026 15:28:24 +0200 Subject: [PATCH 7/9] fix(log-viewer): show step details instead of "no logs found" in flow modal - Convert workflow_steps dict to array format in get_workflow_steps_json so the JS frontend can properly render step data - Add renderStepDetails function to display step metadata, duration, status, and error details when no per-step log entries exist - Fix timestamp sorting to handle None timestamps correctly - Update tests for new workflow steps array format --- webhook_server/tests/test_log_viewer.py | 47 ++++- webhook_server/web/log_viewer.py | 77 +++++++- webhook_server/web/static/js/log_viewer.js | 197 ++++++++++++++++++++- 3 files changed, 300 insertions(+), 21 deletions(-) diff --git a/webhook_server/tests/test_log_viewer.py b/webhook_server/tests/test_log_viewer.py index 98688c38..92e57a0b 100644 --- a/webhook_server/tests/test_log_viewer.py +++ b/webhook_server/tests/test_log_viewer.py @@ -101,8 +101,16 @@ def sample_json_webhook_data(self) -> dict: "duration_ms": 5000, }, "workflow_steps": { - "step1": {"status": "completed", "duration_ms": 1000}, - "step2": {"status": "completed", "duration_ms": 2000}, + "step1": { + "status": "completed", + "duration_ms": 1000, + "timestamp": "2025-01-05T10:00:01.000000Z", + }, + "step2": { + "status": "completed", + "duration_ms": 2000, + "timestamp": "2025-01-05T10:00:03.000000Z", + }, }, "token_spend": 35, "success": True, @@ -284,11 +292,36 @@ async def test_get_workflow_steps_json_returns_workflow_data(self, controller, t assert result["repository"] == "org/test-repo" assert result["sender"] == "test-user" assert result["pr"]["number"] == 456 - assert result["timing"]["duration_ms"] == 5000 - assert result["steps"] == sample_json_webhook_data["workflow_steps"] assert result["token_spend"] == 35 assert result["success"] is True + # Verify steps converted from dict to sorted array + assert result["step_count"] == 2 + assert result["start_time"] == "2025-01-05T10:00:01.000000Z" + assert result["total_duration_ms"] == 5000 # from timing.duration_ms + + steps = result["steps"] + assert isinstance(steps, list) + assert len(steps) == 2 + + # Steps should be sorted by timestamp (step1 before step2) + assert steps[0]["step_name"] == "step1" + assert steps[0]["task_status"] == "completed" + assert steps[0]["duration_ms"] == 1000 + assert steps[0]["message"] == "step1: completed (1000ms)" + assert steps[0]["level"] == "INFO" + assert steps[0]["repository"] == "org/test-repo" + assert steps[0]["event_type"] == "pull_request" + assert steps[0]["pr_number"] == 456 + assert steps[0]["task_id"] == "step1" + assert steps[0]["relative_time_ms"] == 0 + assert steps[0]["step_details"]["status"] == "completed" + + assert steps[1]["step_name"] == "step2" + assert steps[1]["task_status"] == "completed" + assert steps[1]["duration_ms"] == 2000 + assert steps[1]["relative_time_ms"] == 2000 # 2 seconds after step1 + async def test_get_workflow_steps_json_returns_none_for_unknown_hook_id( self, controller, tmp_path, sample_json_webhook_data ): @@ -434,8 +467,10 @@ async def test_get_workflow_steps_json_handles_missing_optional_fields(self, con assert result["repository"] is None assert result["sender"] is None assert result["pr"] is None - assert result["timing"] is None - assert result["steps"] == {} # Default to empty dict + assert result["steps"] == [] # Default to empty list + assert result["step_count"] == 0 + assert result["start_time"] is None + assert result["total_duration_ms"] is None assert result["token_spend"] is None assert result["success"] is None assert result["error"] is None diff --git a/webhook_server/web/log_viewer.py b/webhook_server/web/log_viewer.py index 4d0d0bff..19620f56 100644 --- a/webhook_server/web/log_viewer.py +++ b/webhook_server/web/log_viewer.py @@ -548,13 +548,22 @@ async def get_workflow_steps_json(self, hook_id: str) -> dict[str, Any]: """Get workflow steps directly from JSON logs for a specific hook ID. This is more efficient than parsing text logs since JSON logs contain - the full structured workflow data. + the full structured workflow data. Converts the workflow_steps dict into + an array format matching the structure from _build_workflow_timeline, + so the JavaScript frontend can render it consistently. Args: hook_id: The hook ID to get workflow steps for Returns: - Dictionary with workflow steps from JSON log + Dictionary with: + - hook_id, event_type, action, repository, sender, pr, token_spend, success, error + - steps: array of step objects sorted by timestamp, each with timestamp, + step_name, message, level, repository, event_type, pr_number, task_id, + task_type, task_status, duration_ms, error, relative_time_ms, step_details + - step_count: number of steps + - start_time: earliest step timestamp + - total_duration_ms: total workflow duration Raises: HTTPException: 404 if hook ID not found @@ -563,19 +572,73 @@ async def get_workflow_steps_json(self, hook_id: str) -> dict[str, Any]: # Search JSON logs for this hook_id async for entry in self._stream_json_log_entries(max_files=25, max_entries=50000): if entry.get("hook_id") == hook_id: - # Found the entry - return structured workflow data + # Backward compat: older log files may store these as None + workflow_steps_dict: dict[str, Any] = entry.get("workflow_steps") or {} + timing: dict[str, Any] = entry.get("timing") or {} + repository = entry.get("repository") + event_type = entry.get("event_type") + pr_data = entry.get("pr") or {} + pr_number = pr_data.get("number") + + # Convert workflow_steps dict to sorted array of step objects + steps: list[dict[str, Any]] = [] + for step_name, step_data in workflow_steps_dict.items(): + status = step_data.get("status", "unknown") + duration_ms = step_data.get("duration_ms") + duration_str = f" ({duration_ms}ms)" if duration_ms is not None else "" + steps.append({ + "timestamp": step_data.get("timestamp"), + "step_name": step_name, + "message": f"{step_name}: {status}{duration_str}", + "level": "ERROR" if status == "failed" else "INFO", + "repository": repository, + "event_type": event_type, + "pr_number": pr_number, + "task_id": step_name, + "task_type": None, + "task_status": status, + "duration_ms": duration_ms, + "error": step_data.get("error"), + "relative_time_ms": 0, + "step_details": step_data, + }) + + # Sort steps by timestamp; None-timestamp steps sort to the end + steps.sort(key=lambda s: (s["timestamp"] is None, s["timestamp"] or "")) + + # Parse earliest timestamp once for reuse in both relative_time_ms and total_duration_ms + first_ts: datetime.datetime | None = None + if steps and steps[0]["timestamp"]: + first_ts = datetime.datetime.fromisoformat(steps[0]["timestamp"]) + + # Calculate relative_time_ms from the earliest step + if first_ts is not None: + for step in steps: + if step["timestamp"]: + step_ts = datetime.datetime.fromisoformat(step["timestamp"]) + step["relative_time_ms"] = int((step_ts - first_ts).total_seconds() * 1000) + + # Determine start_time and total_duration_ms + start_time = steps[0]["timestamp"] if steps and steps[0]["timestamp"] else timing.get("started_at") + total_duration_ms = timing.get("duration_ms") + if total_duration_ms is None and first_ts is not None and len(steps) > 1 and steps[-1]["timestamp"]: + last_ts = datetime.datetime.fromisoformat(steps[-1]["timestamp"]) + total_duration_ms = int((last_ts - first_ts).total_seconds() * 1000) + return { "hook_id": hook_id, - "event_type": entry.get("event_type"), + "event_type": event_type, "action": entry.get("action"), - "repository": entry.get("repository"), + "repository": repository, "sender": entry.get("sender"), "pr": entry.get("pr"), - "timing": entry.get("timing"), - "steps": entry.get("workflow_steps") or {}, "token_spend": entry.get("token_spend"), "success": entry.get("success"), "error": entry.get("error"), + "steps": steps, + "step_count": len(steps), + "start_time": start_time, + "total_duration_ms": total_duration_ms, } raise ValueError(f"No JSON log entry found for hook ID: {hook_id}") diff --git a/webhook_server/web/static/js/log_viewer.js b/webhook_server/web/static/js/log_viewer.js index 71bf040d..5fb8681f 100644 --- a/webhook_server/web/static/js/log_viewer.js +++ b/webhook_server/web/static/js/log_viewer.js @@ -1692,20 +1692,206 @@ async function filterByStep(stepIndex) { // Toggle: if this step's logs are already showing, hide them if (logsContainer.style.display === "block") { logsContainer.style.display = "none"; - logsContainer.innerHTML = ""; + logsContainer.replaceChildren(); return; } // Hide all other step logs document.querySelectorAll(".step-logs-container").forEach((container) => { container.style.display = "none"; - container.innerHTML = ""; + container.replaceChildren(); }); // Show logs for this step await showStepLogsInModal(step, logsContainer); } +function renderStepDetails(step, container) { + // --- Metadata card --- + const card = document.createElement("div"); + card.style.cssText = + "background: var(--container-bg); border: 1px solid var(--border-color); border-radius: 8px; padding: 16px; margin-bottom: 12px;"; + + // Header row: step name + status badge + const header = document.createElement("div"); + header.style.cssText = + "display: flex; align-items: center; justify-content: space-between; margin-bottom: 12px;"; + + const stepName = document.createElement("div"); + stepName.style.cssText = + "font-weight: 600; font-size: 15px; color: var(--text-color);"; + stepName.textContent = step.step_name || step.task_id || "Unknown step"; + + const statusBadge = document.createElement("span"); + const status = step.task_status || "unknown"; + const isCompleted = status === "completed"; + const isFailed = status === "failed"; + const badgeColor = isFailed ? "#dc3545" : isCompleted ? "#28a745" : "#ffc107"; + statusBadge.style.cssText = + `padding: 2px 10px; border-radius: 12px; font-size: 12px; font-weight: 600; color: ${isFailed || isCompleted ? "#fff" : "#333"}; background: ` + + badgeColor + + ";"; + statusBadge.textContent = status; + + header.appendChild(stepName); + header.appendChild(statusBadge); + card.appendChild(header); + + // Info grid: duration + timestamp + const infoGrid = document.createElement("div"); + infoGrid.style.cssText = + "display: grid; grid-template-columns: 1fr 1fr; gap: 8px; font-size: 13px;"; + + if (step.duration_ms != null) { + const durationItem = document.createElement("div"); + + const durationLabel = document.createElement("span"); + durationLabel.style.cssText = + "color: var(--timestamp-color); margin-right: 6px;"; + durationLabel.textContent = "Duration:"; + + const durationValue = document.createElement("span"); + durationValue.style.cssText = "font-weight: 600; color: var(--text-color);"; + durationValue.textContent = (step.duration_ms / 1000).toFixed(2) + "s"; + + durationItem.appendChild(durationLabel); + durationItem.appendChild(durationValue); + infoGrid.appendChild(durationItem); + } + + if (step.timestamp) { + const timeItem = document.createElement("div"); + + const timeLabel = document.createElement("span"); + timeLabel.style.cssText = + "color: var(--timestamp-color); margin-right: 6px;"; + timeLabel.textContent = "Timestamp:"; + + const timeValue = document.createElement("span"); + timeValue.style.cssText = "color: var(--text-color);"; + timeValue.textContent = new Date(step.timestamp).toLocaleString(); + + timeItem.appendChild(timeLabel); + timeItem.appendChild(timeValue); + infoGrid.appendChild(timeItem); + } + + card.appendChild(infoGrid); + container.appendChild(card); + + // --- Error details (prominent, if present) --- + if (step.error) { + const errorCard = document.createElement("div"); + errorCard.style.cssText = + "background: var(--log-error-bg); border: 1px solid #dc3545; border-radius: 8px; padding: 16px; margin-bottom: 12px;"; + + const errorTitle = document.createElement("div"); + errorTitle.style.cssText = + "font-weight: 600; color: #dc3545; margin-bottom: 8px; font-size: 14px;"; + errorTitle.textContent = "Error"; + errorCard.appendChild(errorTitle); + + if (typeof step.error === "object") { + const errorFields = ["type", "message", "traceback"]; + errorFields.forEach((field) => { + if (!step.error[field]) return; + + const row = document.createElement("div"); + row.style.cssText = "margin-bottom: 6px;"; + + const label = document.createElement("span"); + label.style.cssText = "font-weight: 600; color: var(--text-color);"; + label.textContent = field.charAt(0).toUpperCase() + field.slice(1) + ": "; + row.appendChild(label); + + if (field === "traceback") { + const pre = document.createElement("pre"); + pre.style.cssText = + "margin: 4px 0 0 0; padding: 8px; background: var(--log-debug-bg); border-radius: 4px; font-size: 12px; overflow-x: auto; white-space: pre-wrap; word-break: break-word; color: var(--text-color);"; + pre.textContent = step.error[field]; + row.appendChild(pre); + } else { + const value = document.createElement("span"); + value.style.cssText = "color: var(--text-color);"; + value.textContent = step.error[field]; + row.appendChild(value); + } + + errorCard.appendChild(row); + }); + } else { + const errorText = document.createElement("div"); + errorText.style.cssText = "color: var(--text-color);"; + errorText.textContent = String(step.error); + errorCard.appendChild(errorText); + } + + container.appendChild(errorCard); + } + + // --- Step details (key-value pairs from step_details) --- + if (step.step_details && typeof step.step_details === "object") { + const skipFields = new Set([ + "timestamp", + "status", + "error", + "duration_ms", + ]); + const detailKeys = Object.keys(step.step_details).filter( + (k) => !skipFields.has(k), + ); + + if (detailKeys.length > 0) { + const detailsCard = document.createElement("div"); + detailsCard.style.cssText = + "background: var(--container-bg); border: 1px solid var(--border-color); border-radius: 8px; padding: 16px;"; + + const detailsTitle = document.createElement("div"); + detailsTitle.style.cssText = + "font-weight: 600; color: var(--text-color); margin-bottom: 10px; font-size: 14px;"; + detailsTitle.textContent = "Step Details"; + detailsCard.appendChild(detailsTitle); + + detailKeys.forEach((key) => { + const value = step.step_details[key]; + const row = document.createElement("div"); + row.style.cssText = + "display: flex; padding: 4px 0; border-bottom: 1px solid var(--log-entry-border); font-size: 13px;"; + + const keyEl = document.createElement("span"); + keyEl.style.cssText = + "color: var(--timestamp-color); min-width: 140px; flex-shrink: 0; font-weight: 500;"; + keyEl.textContent = key; + row.appendChild(keyEl); + + const valueEl = document.createElement("span"); + valueEl.style.cssText = + "color: var(--text-color); word-break: break-word;"; + if (value != null && typeof value === "object") { + const pre = document.createElement("pre"); + pre.style.cssText = + "margin: 0; font-size: 12px; white-space: pre-wrap; word-break: break-word;"; + pre.textContent = JSON.stringify(value, null, 2); + valueEl.appendChild(pre); + } else { + valueEl.textContent = value != null ? String(value) : "null"; + } + + row.appendChild(valueEl); + detailsCard.appendChild(row); + }); + + // Remove border from last row + const lastRow = detailsCard.lastElementChild; + if (lastRow) { + lastRow.style.borderBottom = "none"; + } + + container.appendChild(detailsCard); + } + } +} + async function showStepLogsInModal(step, logsContainer) { if (!logsContainer) return; @@ -1743,12 +1929,7 @@ async function showStepLogsInModal(step, logsContainer) { logsContainer.textContent = ""; if (data.entries.length === 0) { - const emptyMsg = document.createElement("div"); - emptyMsg.textContent = "No logs found for this step"; - emptyMsg.style.textAlign = "center"; - emptyMsg.style.color = "var(--timestamp-color)"; - emptyMsg.style.padding = "12px"; - logsContainer.appendChild(emptyMsg); + renderStepDetails(step, logsContainer); return; } From 6e217e9d73e557688be3b2d35783a29735dc1814 Mon Sep 17 00:00:00 2001 From: rnetser Date: Mon, 16 Feb 2026 16:55:53 +0200 Subject: [PATCH 8/9] feat(log-viewer): add step-logs endpoint and fix step detail display - Add /logs/api/step-logs/{hook_id}/{step_name} endpoint that returns step metadata from structured JSON workflow logs - Replace inline-styled renderStepDetails with CSS class-based version matching deployed server architecture - Remove misleading "No log entries found during this step's execution" message - step details (status, duration, error) are shown instead - Add comprehensive CSS for step details and step log entries - Improve showFlowModal error handling with JSON error detail parsing - Add tests for get_step_logs endpoint (3 test cases) --- webhook_server/app.py | 15 + webhook_server/tests/test_log_viewer.py | 50 +++ webhook_server/web/log_viewer.py | 50 +++ webhook_server/web/static/css/log_viewer.css | 299 +++++++++++++ webhook_server/web/static/js/log_viewer.js | 430 +++++++++---------- 5 files changed, 622 insertions(+), 222 deletions(-) diff --git a/webhook_server/app.py b/webhook_server/app.py index 4ea6a777..4969ef18 100644 --- a/webhook_server/app.py +++ b/webhook_server/app.py @@ -17,6 +17,7 @@ Depends, FastAPI, HTTPException, + Path, Query, Request, Response, @@ -1144,6 +1145,20 @@ async def get_workflow_steps(hook_id: str, controller: LogViewerController = con return await get_workflow_steps_core(controller=controller, hook_id=hook_id) +@FASTAPI_APP.get( + "/logs/api/step-logs/{hook_id}/{step_name}", + operation_id="get_step_logs", + dependencies=[Depends(require_log_server_enabled)], +) +async def get_step_logs( + hook_id: str = Path(min_length=1, max_length=100), + step_name: str = Path(min_length=1, max_length=100), + controller: LogViewerController = controller_dependency, +) -> dict[str, Any]: + """Retrieve detailed log entries for a specific workflow step within a webhook execution.""" + return await controller.get_step_logs(hook_id=hook_id, step_name=step_name) + + @FASTAPI_APP.websocket("/logs/ws") async def websocket_log_stream( websocket: WebSocket, diff --git a/webhook_server/tests/test_log_viewer.py b/webhook_server/tests/test_log_viewer.py index 92e57a0b..efbac565 100644 --- a/webhook_server/tests/test_log_viewer.py +++ b/webhook_server/tests/test_log_viewer.py @@ -766,6 +766,56 @@ async def test_stream_log_entries_empty_json_file(self, controller, tmp_path): # Should yield nothing assert len(entries) == 0 + async def test_get_step_logs_returns_step_data(self, controller, tmp_path, sample_json_webhook_data): + """Test get_step_logs returns step metadata for a valid hook_id and step_name.""" + log_dir = tmp_path / "logs" + log_dir.mkdir() + + # Create JSON log file with workflow steps + self.create_json_log_file(log_dir, "webhooks_2025-01-05.json", [sample_json_webhook_data]) + + # Get step logs for an existing step + result = await controller.get_step_logs("test-hook-123", "step1") + + # Should return step metadata + assert result["step"]["name"] == "step1" + assert result["step"]["status"] == "completed" + assert result["step"]["timestamp"] == "2025-01-05T10:00:01.000000Z" + assert result["step"]["duration_ms"] == 1000 + assert result["step"]["error"] is None + assert result["logs"] == [] + assert result["log_count"] == 0 + + async def test_get_step_logs_hook_not_found(self, controller, tmp_path, sample_json_webhook_data): + """Test get_step_logs raises HTTPException 404 when hook_id doesn't exist.""" + log_dir = tmp_path / "logs" + log_dir.mkdir() + + # Create JSON log file with a different hook_id + self.create_json_log_file(log_dir, "webhooks_2025-01-05.json", [sample_json_webhook_data]) + + # Try to get step logs for non-existent hook_id + with pytest.raises(HTTPException) as exc: + await controller.get_step_logs("non-existent-hook", "step1") + + assert exc.value.status_code == 404 + assert "non-existent-hook" in str(exc.value.detail) + + async def test_get_step_logs_step_not_found(self, controller, tmp_path, sample_json_webhook_data): + """Test get_step_logs raises HTTPException 404 when step_name doesn't exist.""" + log_dir = tmp_path / "logs" + log_dir.mkdir() + + # Create JSON log file + self.create_json_log_file(log_dir, "webhooks_2025-01-05.json", [sample_json_webhook_data]) + + # Try to get step logs for non-existent step name + with pytest.raises(HTTPException) as exc: + await controller.get_step_logs("test-hook-123", "non_existent_step") + + assert exc.value.status_code == 404 + assert "non_existent_step" in str(exc.value.detail) + class TestLogViewerGetLogEntries: """Test cases for get_log_entries method.""" diff --git a/webhook_server/web/log_viewer.py b/webhook_server/web/log_viewer.py index 19620f56..11c2eb3d 100644 --- a/webhook_server/web/log_viewer.py +++ b/webhook_server/web/log_viewer.py @@ -759,6 +759,56 @@ async def get_workflow_steps(self, hook_id: str) -> dict[str, Any]: self.logger.error(f"Error getting workflow steps: {e}") raise HTTPException(status_code=500, detail="Internal server error") from e + async def get_step_logs(self, hook_id: str, step_name: str) -> dict[str, Any]: + """Get detailed log entries for a specific workflow step within a webhook execution. + + Args: + hook_id: The hook ID to look up + step_name: The workflow step name to retrieve + + Returns: + Dictionary with step metadata and logs array + + Raises: + HTTPException: 404 if hook_id or step_name not found, 500 on internal error + """ + try: + async for entry in self._stream_json_log_entries(max_files=25, max_entries=50000): + if entry.get("hook_id") == hook_id: + workflow_steps_dict: dict[str, Any] = entry.get("workflow_steps") or {} + if step_name not in workflow_steps_dict: + raise HTTPException( + status_code=404, + detail=f"Step '{step_name}' not found in workflow data for hook ID: {hook_id}", + ) + + step_data = workflow_steps_dict[step_name] + return { + "step": { + "name": step_name, + "status": step_data.get("status", "unknown"), + "timestamp": step_data.get("timestamp"), + "duration_ms": step_data.get("duration_ms"), + "error": step_data.get("error"), + }, + "logs": [], + "log_count": 0, + } + + raise HTTPException( + status_code=404, + detail=f"No log entry found for hook ID: {hook_id}", + ) + + except asyncio.CancelledError: + self.logger.debug("Operation cancelled") + raise + except HTTPException: + raise + except Exception as e: + self.logger.error(f"Error getting step logs: {e}") + raise HTTPException(status_code=500, detail="Internal server error") from e + def _build_workflow_timeline(self, workflow_steps: list[LogEntry], hook_id: str) -> dict[str, Any]: """Build timeline data from workflow step entries. diff --git a/webhook_server/web/static/css/log_viewer.css b/webhook_server/web/static/css/log_viewer.css index ccaca2ce..ad337980 100644 --- a/webhook_server/web/static/css/log_viewer.css +++ b/webhook_server/web/static/css/log_viewer.css @@ -1151,3 +1151,302 @@ body { .task-group-steps .flow-step-container.nested::before { left: -1px; } + +/* Step Details Styles - for workflow step metadata display */ +.step-details-entry { + background: var(--container-bg); + border: 1px solid var(--border-color); + border-radius: 8px; + padding: 16px; + font-family: monospace; + font-size: 13px; +} + +.step-details-header { + display: flex; + align-items: center; + gap: 16px; + margin-bottom: 12px; + padding-bottom: 12px; + border-bottom: 1px solid var(--border-color); +} + +.step-details-timestamp { + color: var(--timestamp-color); + font-size: 12px; +} + +.step-details-name { + font-weight: 700; + font-size: 15px; + color: var(--text-color); +} + +.step-details-status-row { + display: flex; + align-items: center; + gap: 16px; + margin-bottom: 12px; +} + +.step-details-badge { + padding: 4px 12px; + border-radius: 4px; + font-size: 11px; + font-weight: 700; + text-transform: uppercase; + letter-spacing: 0.5px; +} + +.step-status-completed, +.step-status-success { + background: rgba(40, 167, 69, 0.15); + color: #28a745; + border: 1px solid rgba(40, 167, 69, 0.3); +} + +.step-status-failed, +.step-status-error { + background: rgba(220, 53, 69, 0.15); + color: #dc3545; + border: 1px solid rgba(220, 53, 69, 0.3); +} + +.step-status-in_progress, +.step-status-processing, +.step-status-step { + background: rgba(0, 123, 255, 0.15); + color: #007bff; + border: 1px solid rgba(0, 123, 255, 0.3); +} + +.step-status-warning { + background: rgba(255, 193, 7, 0.15); + color: #856404; + border: 1px solid rgba(255, 193, 7, 0.3); +} + +.step-status-info { + background: var(--level-info-bg); + color: var(--level-info-border); + border: 1px solid var(--level-info-border); +} + +.step-details-duration { + color: var(--level-info-border); + font-weight: 600; + font-size: 13px; +} + +.step-details-message { + color: var(--text-color); + padding: 12px; + background: var(--log-debug-bg); + border-radius: 4px; + margin-bottom: 12px; + word-break: break-word; + line-height: 1.5; +} + +.step-details-error { + background: rgba(220, 53, 69, 0.1); + border: 1px solid rgba(220, 53, 69, 0.3); + border-left: 4px solid #dc3545; + padding: 12px; + border-radius: 4px; + margin-bottom: 12px; +} + +.step-details-error-label { + color: #dc3545; + font-weight: 700; +} + +.step-details-error-text { + color: var(--text-color); + word-break: break-word; +} + +.step-details-metadata { + display: flex; + flex-wrap: wrap; + gap: 8px; + margin-top: 8px; +} + +.step-details-metadata span { + padding: 4px 8px; + background: var(--tag-bg); + border-radius: 4px; + font-size: 12px; + color: var(--text-secondary); +} + +.step-meta-repository { + color: var(--level-info-border) !important; +} + +.step-meta-pr_number { + color: #28a745 !important; +} + +.step-meta-github_user { + color: #6f42c1 !important; +} + +.step-meta-hook_id { + color: var(--timestamp-color) !important; +} + +/* Flow in-progress status */ +.flow-in-progress { + background: rgba(0, 123, 255, 0.1); + border: 1px solid #007bff; + border-left: 4px solid #007bff; + padding: 16px; + border-radius: 8px; + text-align: center; + margin-top: 24px; +} + +.flow-in-progress h3 { + color: #007bff; + margin: 0; + font-size: 18px; +} + +/* Step Logs Styles - for fetched log entries within workflow steps */ +.step-logs-loading { + padding: 16px; + text-align: center; + color: var(--timestamp-color); + font-style: italic; + background: var(--log-debug-bg); + border-radius: 4px; + margin-top: 12px; +} + +.step-logs-list { + margin-top: 16px; + border: 1px solid var(--border-color); + border-radius: 8px; + overflow: hidden; + background: var(--container-bg); +} + +.step-logs-header { + padding: 10px 16px; + background: var(--tag-bg); + border-bottom: 1px solid var(--border-color); + font-weight: 600; + font-size: 13px; + color: var(--text-secondary); +} + +.step-log-entry { + display: flex; + align-items: flex-start; + gap: 12px; + padding: 10px 16px; + border-bottom: 1px solid var(--log-entry-border); + font-family: monospace; + font-size: 12px; + transition: background-color 0.2s ease; +} + +.step-log-entry:last-child { + border-bottom: none; +} + +.step-log-entry:hover { + background-color: var(--log-entry-border); +} + +.step-log-entry.log-level-error { + background-color: rgba(220, 53, 69, 0.05); +} + +.step-log-entry.log-level-warning { + background-color: rgba(255, 193, 7, 0.05); +} + +.step-log-entry.log-level-success { + background-color: rgba(40, 167, 69, 0.05); +} + +.step-log-timestamp { + flex-shrink: 0; + width: 80px; + color: var(--timestamp-color); + font-size: 11px; +} + +.step-log-level { + flex-shrink: 0; + width: 60px; + padding: 2px 6px; + border-radius: 3px; + font-size: 10px; + font-weight: 700; + text-align: center; + text-transform: uppercase; +} + +.log-level-badge-info { + background: var(--level-info-bg); + color: var(--level-info-border); +} + +.log-level-badge-error { + background: rgba(220, 53, 69, 0.15); + color: #dc3545; +} + +.log-level-badge-warning { + background: rgba(255, 193, 7, 0.15); + color: #856404; +} + +.log-level-badge-debug { + background: var(--tag-bg); + color: var(--text-secondary); +} + +.log-level-badge-step { + background: rgba(0, 123, 255, 0.15); + color: #007bff; +} + +.log-level-badge-success { + background: rgba(40, 167, 69, 0.15); + color: #28a745; +} + +.step-log-message { + flex: 1; + word-break: break-word; + overflow-wrap: anywhere; + color: var(--text-color); + line-height: 1.4; +} + +.step-logs-empty { + padding: 16px; + text-align: center; + color: var(--timestamp-color); + font-style: italic; + background: var(--log-debug-bg); + border-radius: 4px; + margin-top: 12px; +} + +.step-logs-error { + padding: 12px 16px; + margin-top: 12px; + background: rgba(220, 53, 69, 0.1); + border: 1px solid rgba(220, 53, 69, 0.3); + border-left: 4px solid #dc3545; + border-radius: 4px; + color: #dc3545; + font-size: 13px; +} diff --git a/webhook_server/web/static/js/log_viewer.js b/webhook_server/web/static/js/log_viewer.js index 5fb8681f..5de49359 100644 --- a/webhook_server/web/static/js/log_viewer.js +++ b/webhook_server/web/static/js/log_viewer.js @@ -839,14 +839,40 @@ function showFlowModal(hookId) { fetch(`/logs/api/workflow-steps/${encodeURIComponent(hookId)}`, { signal: currentFlowController.signal, }) - .then((response) => { + .then(async (response) => { if (!response.ok) { - if (response.status === 404) { - console.log("No flow data found for hook ID:", hookId); - showFlowModalError("No workflow data found for this hook"); + const status = response.status; + + // Try to parse error detail from JSON response + let errorDetail = null; + try { + const errorData = await response.json(); + errorDetail = errorData.detail; + } catch { + // JSON parsing failed, use default messages + } + + if (status === 404) { + const message = errorDetail || "No workflow data found for this hook"; + console.log("No flow data found for hook ID:", hookId, message); + showFlowModalError(message); + return; + } else if (status === 400) { + const message = errorDetail || "Invalid request"; + console.error("Bad request for hook ID:", hookId, message); + showFlowModalError(message); + return; + } else if (status >= 500) { + console.error("Server error for hook ID:", hookId, errorDetail); + showFlowModalError("Server error occurred. Please try again later."); + return; + } else { + const message = + errorDetail || `HTTP ${status}: ${response.statusText}`; + console.error("Error fetching flow data:", message); + showFlowModalError(message); return; } - throw new Error(`HTTP ${response.status}: ${response.statusText}`); } return response.json(); }) @@ -1706,248 +1732,208 @@ async function filterByStep(stepIndex) { await showStepLogsInModal(step, logsContainer); } -function renderStepDetails(step, container) { - // --- Metadata card --- - const card = document.createElement("div"); - card.style.cssText = - "background: var(--container-bg); border: 1px solid var(--border-color); border-radius: 8px; padding: 16px; margin-bottom: 12px;"; +/** + * Renders step details as a formatted log-like entry. + * Displays the step's own metadata (status, duration, error) instead of searching logs. + * + * @param {Object} step - The step object from workflow_steps + * @returns {HTMLElement} - The rendered step details element + */ +function renderStepDetails(step) { + const detailsContainer = document.createElement("div"); + detailsContainer.className = "step-details-entry"; + + // Header row with timestamp and step name + const headerRow = document.createElement("div"); + headerRow.className = "step-details-header"; - // Header row: step name + status badge - const header = document.createElement("div"); - header.style.cssText = - "display: flex; align-items: center; justify-content: space-between; margin-bottom: 12px;"; + if (step.timestamp) { + const timestampSpan = document.createElement("span"); + timestampSpan.className = "step-details-timestamp"; + timestampSpan.textContent = new Date(step.timestamp).toLocaleString(); + headerRow.appendChild(timestampSpan); + } - const stepName = document.createElement("div"); - stepName.style.cssText = - "font-weight: 600; font-size: 15px; color: var(--text-color);"; - stepName.textContent = step.step_name || step.task_id || "Unknown step"; + const stepNameSpan = document.createElement("span"); + stepNameSpan.className = "step-details-name"; + const stepName = step.step_name || "unknown_step"; + stepNameSpan.textContent = stepName; + headerRow.appendChild(stepNameSpan); - const statusBadge = document.createElement("span"); - const status = step.task_status || "unknown"; - const isCompleted = status === "completed"; - const isFailed = status === "failed"; - const badgeColor = isFailed ? "#dc3545" : isCompleted ? "#28a745" : "#ffc107"; - statusBadge.style.cssText = - `padding: 2px 10px; border-radius: 12px; font-size: 12px; font-weight: 600; color: ${isFailed || isCompleted ? "#fff" : "#333"}; background: ` + - badgeColor + - ";"; - statusBadge.textContent = status; - - header.appendChild(stepName); - header.appendChild(statusBadge); - card.appendChild(header); - - // Info grid: duration + timestamp - const infoGrid = document.createElement("div"); - infoGrid.style.cssText = - "display: grid; grid-template-columns: 1fr 1fr; gap: 8px; font-size: 13px;"; - - if (step.duration_ms != null) { - const durationItem = document.createElement("div"); - - const durationLabel = document.createElement("span"); - durationLabel.style.cssText = - "color: var(--timestamp-color); margin-right: 6px;"; - durationLabel.textContent = "Duration:"; - - const durationValue = document.createElement("span"); - durationValue.style.cssText = "font-weight: 600; color: var(--text-color);"; - durationValue.textContent = (step.duration_ms / 1000).toFixed(2) + "s"; - - durationItem.appendChild(durationLabel); - durationItem.appendChild(durationValue); - infoGrid.appendChild(durationItem); - } + detailsContainer.appendChild(headerRow); - if (step.timestamp) { - const timeItem = document.createElement("div"); - - const timeLabel = document.createElement("span"); - timeLabel.style.cssText = - "color: var(--timestamp-color); margin-right: 6px;"; - timeLabel.textContent = "Timestamp:"; - - const timeValue = document.createElement("span"); - timeValue.style.cssText = "color: var(--text-color);"; - timeValue.textContent = new Date(step.timestamp).toLocaleString(); - - timeItem.appendChild(timeLabel); - timeItem.appendChild(timeValue); - infoGrid.appendChild(timeItem); - } - - card.appendChild(infoGrid); - container.appendChild(card); - - // --- Error details (prominent, if present) --- - if (step.error) { - const errorCard = document.createElement("div"); - errorCard.style.cssText = - "background: var(--log-error-bg); border: 1px solid #dc3545; border-radius: 8px; padding: 16px; margin-bottom: 12px;"; - - const errorTitle = document.createElement("div"); - errorTitle.style.cssText = - "font-weight: 600; color: #dc3545; margin-bottom: 8px; font-size: 14px;"; - errorTitle.textContent = "Error"; - errorCard.appendChild(errorTitle); - - if (typeof step.error === "object") { - const errorFields = ["type", "message", "traceback"]; - errorFields.forEach((field) => { - if (!step.error[field]) return; - - const row = document.createElement("div"); - row.style.cssText = "margin-bottom: 6px;"; - - const label = document.createElement("span"); - label.style.cssText = "font-weight: 600; color: var(--text-color);"; - label.textContent = field.charAt(0).toUpperCase() + field.slice(1) + ": "; - row.appendChild(label); - - if (field === "traceback") { - const pre = document.createElement("pre"); - pre.style.cssText = - "margin: 4px 0 0 0; padding: 8px; background: var(--log-debug-bg); border-radius: 4px; font-size: 12px; overflow-x: auto; white-space: pre-wrap; word-break: break-word; color: var(--text-color);"; - pre.textContent = step.error[field]; - row.appendChild(pre); - } else { - const value = document.createElement("span"); - value.style.cssText = "color: var(--text-color);"; - value.textContent = step.error[field]; - row.appendChild(value); - } + // Status row + const statusRow = document.createElement("div"); + statusRow.className = "step-details-status-row"; - errorCard.appendChild(row); - }); + const statusBadge = document.createElement("span"); + const status = step.task_status || step.step_status || step.level || "INFO"; + statusBadge.className = `step-details-badge step-status-${status.toLowerCase()}`; + statusBadge.textContent = status.toUpperCase(); + statusRow.appendChild(statusBadge); + + const durationMs = step.step_duration_ms || step.duration_ms; + if (durationMs !== undefined && durationMs !== null) { + const durationSpan = document.createElement("span"); + durationSpan.className = "step-details-duration"; + if (durationMs >= 1000) { + durationSpan.textContent = `Duration: ${(durationMs / 1000).toFixed(2)}s`; } else { - const errorText = document.createElement("div"); - errorText.style.cssText = "color: var(--text-color);"; - errorText.textContent = String(step.error); - errorCard.appendChild(errorText); + durationSpan.textContent = `Duration: ${durationMs}ms`; } - - container.appendChild(errorCard); + statusRow.appendChild(durationSpan); } - // --- Step details (key-value pairs from step_details) --- - if (step.step_details && typeof step.step_details === "object") { - const skipFields = new Set([ - "timestamp", - "status", - "error", - "duration_ms", - ]); - const detailKeys = Object.keys(step.step_details).filter( - (k) => !skipFields.has(k), - ); + detailsContainer.appendChild(statusRow); - if (detailKeys.length > 0) { - const detailsCard = document.createElement("div"); - detailsCard.style.cssText = - "background: var(--container-bg); border: 1px solid var(--border-color); border-radius: 8px; padding: 16px;"; - - const detailsTitle = document.createElement("div"); - detailsTitle.style.cssText = - "font-weight: 600; color: var(--text-color); margin-bottom: 10px; font-size: 14px;"; - detailsTitle.textContent = "Step Details"; - detailsCard.appendChild(detailsTitle); - - detailKeys.forEach((key) => { - const value = step.step_details[key]; - const row = document.createElement("div"); - row.style.cssText = - "display: flex; padding: 4px 0; border-bottom: 1px solid var(--log-entry-border); font-size: 13px;"; - - const keyEl = document.createElement("span"); - keyEl.style.cssText = - "color: var(--timestamp-color); min-width: 140px; flex-shrink: 0; font-weight: 500;"; - keyEl.textContent = key; - row.appendChild(keyEl); - - const valueEl = document.createElement("span"); - valueEl.style.cssText = - "color: var(--text-color); word-break: break-word;"; - if (value != null && typeof value === "object") { - const pre = document.createElement("pre"); - pre.style.cssText = - "margin: 0; font-size: 12px; white-space: pre-wrap; word-break: break-word;"; - pre.textContent = JSON.stringify(value, null, 2); - valueEl.appendChild(pre); - } else { - valueEl.textContent = value != null ? String(value) : "null"; - } + // Message (if different from step_name) + if (step.message && step.message !== stepName) { + const messageRow = document.createElement("div"); + messageRow.className = "step-details-message"; + messageRow.textContent = step.message; + detailsContainer.appendChild(messageRow); + } - row.appendChild(valueEl); - detailsCard.appendChild(row); - }); + // Error details (if step failed) + const stepError = step.step_error || step.error; + if (stepError) { + const errorRow = document.createElement("div"); + errorRow.className = "step-details-error"; - // Remove border from last row - const lastRow = detailsCard.lastElementChild; - if (lastRow) { - lastRow.style.borderBottom = "none"; - } + const errorLabel = document.createElement("span"); + errorLabel.className = "step-details-error-label"; + errorLabel.textContent = "Error: "; + errorRow.appendChild(errorLabel); - container.appendChild(detailsCard); + const errorText = document.createElement("span"); + errorText.className = "step-details-error-text"; + let errorMessage; + if (typeof stepError === "string") { + errorMessage = stepError; + } else if (stepError.message) { + errorMessage = stepError.message; + } else { + errorMessage = JSON.stringify(stepError); } + errorText.textContent = errorMessage; + errorRow.appendChild(errorText); + + detailsContainer.appendChild(errorRow); + } + + // Additional metadata + const metadataFields = ["repository", "pr_number", "github_user", "hook_id"]; + const metadataRow = document.createElement("div"); + metadataRow.className = "step-details-metadata"; + + metadataFields.forEach((field) => { + if (step[field]) { + const metaSpan = document.createElement("span"); + metaSpan.className = `step-meta-${field}`; + if (field === "pr_number") { + metaSpan.textContent = `[PR: #${step[field]}]`; + } else if (field === "github_user") { + metaSpan.textContent = `[User: ${step[field]}]`; + } else if (field === "hook_id") { + metaSpan.textContent = `[Hook: ${step[field]}]`; + } else { + metaSpan.textContent = `[${step[field]}]`; + } + metadataRow.appendChild(metaSpan); + } + }); + + if (metadataRow.children.length > 0) { + detailsContainer.appendChild(metadataRow); } + + return detailsContainer; } async function showStepLogsInModal(step, logsContainer) { if (!logsContainer) return; - // Show loading state - logsContainer.style.display = "block"; - logsContainer.textContent = "Loading logs..."; - - // Cancel previous fetch if still in progress + // Cancel any previous step logs fetch if (currentStepLogsController) { currentStepLogsController.abort(); } - - // Create new AbortController for this fetch currentStepLogsController = new AbortController(); - try { - // Using full message for precision to avoid ambiguous matches - const searchText = step.message; - const hookId = currentFlowData.hook_id; - - const params = new URLSearchParams({ - hook_id: hookId, - search: searchText, - limit: "100", - }); - - const response = await fetch(`/logs/api/entries?${params}`, { - signal: currentStepLogsController.signal, - }); - if (!response.ok) throw new Error("Failed to fetch logs"); - - const data = await response.json(); - - // Clear and display logs using safe DOM methods - logsContainer.textContent = ""; - - if (data.entries.length === 0) { - renderStepDetails(step, logsContainer); - return; - } - - // Render log entries - data.entries.forEach((entry) => { - // Reuse the main log entry creator for consistency - const logEntry = createLogEntryElement(entry); - logsContainer.appendChild(logEntry); - }); - - // Scroll to the logs container - logsContainer.scrollIntoView({ behavior: "smooth", block: "nearest" }); - } catch (error) { - if (error.name === "AbortError") { - // Request was cancelled, ignore silently - return; + // Show the container and display step details immediately + logsContainer.style.display = "block"; + logsContainer.textContent = ""; + + // Render the step's own data first - this is the primary information + const stepDetails = renderStepDetails(step); + logsContainer.appendChild(stepDetails); + + // Fetch actual log entries for this step + const stepName = step.step_name; + const hookId = currentFlowData?.hook_id; + + if (stepName && hookId) { + try { + const response = await fetch( + `/logs/api/step-logs/${encodeURIComponent(hookId)}/${encodeURIComponent(stepName)}`, + { signal: currentStepLogsController.signal }, + ); + + if (response.ok) { + const data = await response.json(); + if (data.logs && data.logs.length > 0) { + // Create container for logs + const logsDiv = document.createElement("div"); + logsDiv.className = "step-logs-list"; + + // Add header + const header = document.createElement("div"); + header.className = "step-logs-header"; + header.textContent = `Log entries during step (${data.log_count} found)`; + logsDiv.appendChild(header); + + // Render each log entry + data.logs.forEach((log) => { + const logEntry = document.createElement("div"); + const levelLower = (log.level || "info").toLowerCase(); + logEntry.className = `step-log-entry log-level-${levelLower}`; + + const timestamp = document.createElement("span"); + timestamp.className = "step-log-timestamp"; + timestamp.textContent = new Date(log.timestamp).toLocaleTimeString(); + + const level = document.createElement("span"); + level.className = `step-log-level log-level-badge-${levelLower}`; + level.textContent = log.level || "INFO"; + + const message = document.createElement("span"); + message.className = "step-log-message"; + message.textContent = log.message; + + logEntry.appendChild(timestamp); + logEntry.appendChild(level); + logEntry.appendChild(message); + logsDiv.appendChild(logEntry); + }); + + logsContainer.appendChild(logsDiv); + } + // When logs are empty: step details are already shown above - no message needed + } else if (response.status !== 404) { + // Only show error for non-404 failures (404 just means no extra logs available) + const errorDiv = document.createElement("div"); + errorDiv.className = "step-logs-error"; + errorDiv.textContent = `Could not load logs: ${response.status}`; + logsContainer.appendChild(errorDiv); + } + } catch (error) { + if (error.name === "AbortError") { + return; + } + // Network errors are non-critical - step details are already shown + console.error("Error fetching step logs:", error); } - console.error("Error fetching step logs:", error); - logsContainer.textContent = "Error loading logs"; } + + // Scroll to the logs container + logsContainer.scrollIntoView({ behavior: "smooth", block: "nearest" }); } From 16f64fa43c59d7de417f3170b94c25a1ddb155c0 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 17 Feb 2026 16:48:15 +0200 Subject: [PATCH 9/9] fix(github-api): catch all exceptions in token validation, not just GithubException asyncio.to_thread can raise ConnectionError, OSError, etc. in addition to GithubException. Since asyncio.gather cancels sibling tasks on the first unhandled exception, a network failure on one token would prevent all other tokens from being validated. Broadening to Exception ensures each token check is fully isolated. --- webhook_server/libs/github_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index aa114dd5..e070167d 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -621,7 +621,7 @@ async def check_token(api: github.Github, token: str) -> str | None: token_suffix = f"...{token[-4:]}" if token else "unknown" try: rate_limit_remaining = await asyncio.to_thread(lambda: api.rate_limiting[-1]) - except GithubException as ex: + except Exception as ex: self.logger.warning( f"{self.log_prefix} Failed to get API rate limit for token ending in '{token_suffix}', " f"skipping. {ex}" @@ -637,7 +637,7 @@ async def check_token(api: github.Github, token: str) -> str | None: try: _api_user = await asyncio.to_thread(lambda: api.get_user().login) - except GithubException as ex: + except Exception as ex: self.logger.exception( f"{self.log_prefix} Failed to get API user for token ending in '{token_suffix}', skipping. {ex}" )