-
Notifications
You must be signed in to change notification settings - Fork 21
refactor exception display #707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,29 +1,33 @@ | ||
| import hashlib | ||
| import json | ||
| import os | ||
| import re | ||
| import sys | ||
| import threading | ||
| import time | ||
| from contextlib import contextmanager | ||
| from dataclasses import dataclass | ||
| from pathlib import Path, PosixPath | ||
| from queue import Queue | ||
| from urllib.parse import urlparse | ||
|
|
||
| import click | ||
| import pexpect | ||
| import requests | ||
| from jumpstarter_driver_composite.client import CompositeClient | ||
| from jumpstarter_driver_opendal.client import FlasherClient, OpendalClient, operator_for_path | ||
| from jumpstarter_driver_opendal.common import PathBuf | ||
| from jumpstarter_driver_pyserial.client import Console | ||
| from opendal import Metadata, Operator | ||
|
|
||
| from jumpstarter_driver_flashers.bundle import FlasherBundleManifestV1Alpha1 | ||
|
|
||
| from jumpstarter.client.decorators import driver_click_group | ||
| from jumpstarter.common.exceptions import ArgumentError, JumpstarterException | ||
| from jumpstarter_cli_common.exceptions import leaf_exceptions | ||
| from typing import TypeVar, Type | ||
|
|
||
| E = TypeVar('E', bound=Exception) | ||
|
Check failure on line 30 in packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
|
||
|
|
||
|
|
||
| class FlashError(JumpstarterException): | ||
|
|
@@ -201,82 +205,64 @@ | |
| raise FlashError(f"Flash operation failed: {non_retryable_error}") from e | ||
| else: | ||
| # Unexpected error, don't retry | ||
| self.logger.error(f"Flash operation failed with unexpected error: {e}") | ||
| raise FlashError(f"Flash operation failed: {e}") from e | ||
| # If it's an ExceptionGroup, show leaf exceptions for better diagnostics | ||
| if isinstance(e, BaseExceptionGroup): | ||
| leaves = leaf_exceptions(e, fix_tracebacks=False) | ||
| error_details = "; ".join([f"{type(exc).__name__}: {exc}" for exc in leaves]) | ||
| self.logger.error(f"Flash operation failed with unexpected error(s): {error_details}") | ||
| raise FlashError(f"Flash operation failed: {error_details}") from e | ||
| else: | ||
| self.logger.error(f"Flash operation failed with unexpected error: {e}") | ||
| raise FlashError(f"Flash operation failed: {e}") from e | ||
|
|
||
|
|
||
| total_time = time.time() - start_time | ||
| # total time in minutes:seconds | ||
| minutes, seconds = divmod(total_time, 60) | ||
| self.logger.info(f"Flashing completed in {int(minutes)}m {int(seconds):02d}s") | ||
|
|
||
| def _get_retryable_error(self, exception: Exception) -> FlashRetryableError | None: | ||
| """Find a retryable error in an exception (or any of its causes). | ||
| def _find_exception_in_chain(self, exception: Exception, exc_type: Type[E]) -> E | None: | ||
| """Find a specific exception type in an exception chain, including ExceptionGroups. | ||
|
|
||
| Args: | ||
| exception: The exception to check | ||
| exception: The exception to search | ||
| exc_type: The exception type to find | ||
|
|
||
| Returns: | ||
| The FlashRetryableError if found, None otherwise | ||
| The matching exception if found, None otherwise | ||
| """ | ||
| # Check if this is an ExceptionGroup and look through its exceptions | ||
| if hasattr(exception, 'exceptions'): | ||
| for sub_exc in exception.exceptions: | ||
| result = self._get_retryable_error(sub_exc) | ||
| # Check if this is a BaseExceptionGroup and look through leaf exceptions | ||
| if isinstance(exception, BaseExceptionGroup): | ||
| for sub_exc in leaf_exceptions(exception, fix_tracebacks=False): | ||
| result = self._find_exception_in_chain(sub_exc, exc_type) | ||
| if result is not None: | ||
| return result | ||
|
|
||
| # Check the current exception | ||
| if isinstance(exception, FlashRetryableError): | ||
| if isinstance(exception, exc_type): | ||
| return exception | ||
|
|
||
| # Check the cause chain | ||
| current = getattr(exception, '__cause__', None) | ||
| while current is not None: | ||
| if isinstance(current, FlashRetryableError): | ||
| if isinstance(current, exc_type): | ||
| return current | ||
| # Also check if the cause is an ExceptionGroup | ||
| if hasattr(current, 'exceptions'): | ||
| for sub_exc in current.exceptions: | ||
| result = self._get_retryable_error(sub_exc) | ||
| # Also check if the cause is a BaseExceptionGroup | ||
| if isinstance(current, BaseExceptionGroup): | ||
| for sub_exc in leaf_exceptions(current, fix_tracebacks=False): | ||
| result = self._find_exception_in_chain(sub_exc, exc_type) | ||
| if result is not None: | ||
| return result | ||
| current = getattr(current, '__cause__', None) | ||
| return None | ||
|
Comment on lines
+224
to
257
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type hint may be too narrow for BaseExceptionGroup. The method signature declares Apply this diff to fix the type hint: - def _find_exception_in_chain(self, exception: Exception, exc_type: Type[E]) -> E | None:
+ def _find_exception_in_chain(self, exception: BaseException, exc_type: Type[E]) -> E | None:
"""Find a specific exception type in an exception chain, including ExceptionGroups.
Args:Also update the callers to accept - def _get_retryable_error(self, exception: Exception) -> FlashRetryableError | None:
+ def _get_retryable_error(self, exception: BaseException) -> FlashRetryableError | None:
"""Find a retryable error in an exception (or any of its causes)."""
return self._find_exception_in_chain(exception, FlashRetryableError)
- def _get_non_retryable_error(self, exception: Exception) -> FlashNonRetryableError | None:
+ def _get_non_retryable_error(self, exception: BaseException) -> FlashNonRetryableError | None:
"""Find a non-retryable error in an exception (or any of its causes)."""
return self._find_exception_in_chain(exception, FlashNonRetryableError)
🤖 Prompt for AI Agents |
||
|
|
||
| def _get_non_retryable_error(self, exception: Exception) -> FlashNonRetryableError | None: | ||
| """Find a non-retryable error in an exception (or any of its causes). | ||
|
|
||
| Args: | ||
| exception: The exception to check | ||
|
|
||
| Returns: | ||
| The FlashNonRetryableError if found, None otherwise | ||
| """ | ||
| # Check if this is an ExceptionGroup and look through its exceptions | ||
| if hasattr(exception, 'exceptions'): | ||
| for sub_exc in exception.exceptions: | ||
| result = self._get_non_retryable_error(sub_exc) | ||
| if result is not None: | ||
| return result | ||
|
|
||
| # Check the current exception | ||
| if isinstance(exception, FlashNonRetryableError): | ||
| return exception | ||
| def _get_retryable_error(self, exception: Exception) -> FlashRetryableError | None: | ||
| """Find a retryable error in an exception (or any of its causes).""" | ||
| return self._find_exception_in_chain(exception, FlashRetryableError) | ||
|
|
||
| # Check the cause chain | ||
| current = getattr(exception, '__cause__', None) | ||
| while current is not None: | ||
| if isinstance(current, FlashNonRetryableError): | ||
| return current | ||
| # Also check if the cause is an ExceptionGroup | ||
| if hasattr(current, 'exceptions'): | ||
| for sub_exc in current.exceptions: | ||
| result = self._get_non_retryable_error(sub_exc) | ||
| if result is not None: | ||
| return result | ||
| current = getattr(current, '__cause__', None) | ||
| return None | ||
| def _get_non_retryable_error(self, exception: Exception) -> FlashNonRetryableError | None: | ||
| """Find a non-retryable error in an exception (or any of its causes).""" | ||
| return self._find_exception_in_chain(exception, FlashNonRetryableError) | ||
|
||
|
|
||
| def _perform_flash_operation( | ||
| self, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix import ordering to resolve pipeline failure.
Standard library imports must come before third-party/project imports per PEP 8. The
typingimports should be moved up.Apply this diff to fix the import order:
🤖 Prompt for AI Agents