Skip to content

Conversation

@hovu96
Copy link
Contributor

@hovu96 hovu96 commented Oct 29, 2025

Summary

Implements a comprehensive exception handling solution that ensures users never see Python stack traces when running CLI commands. The decorator catches all exceptions during command initialization and dependency injection, providing clear, actionable error messages instead.

Scope of Solution

Two-Tier Exception Handling Strategy

  1. Specific Handlers for Well-Known Exceptions

    • Network errors (ClientConnectorError, ClientConnectionError)
    • Timeout errors (TimeoutError)
    • Server disconnect errors (ServerDisconnectedError)
    • SSL/Certificate errors (ClientSSLError, ssl.SSLError)
    • Each handler provides context-specific guidance and troubleshooting steps
  2. Generic Catch-All for Everything Else

    • Ensures NO exception becomes a stack trace
    • Shows error type and message without implementation details
    • Trusts that error messages contain relevant context (e.g., "run workato init")
    • Clean output without redundant advice

Key Design Principles

  • Conservative approach: Only apply specific handlers when confident about error type
  • Generic fallback: Ensures complete coverage - no stack traces slip through
  • Clean messages: Original error messages already contain helpful context, no need to add redundant suggestions
  • Decorator placement: Applied above @inject to catch dependency injection errors

Changes

  • ✅ Add handle_cli_exceptions decorator to exception_handler.py
  • ✅ Apply decorator to 11 commands that use dependency injection
  • ✅ Add 16 comprehensive unit tests covering sync/async functions and all error types
  • ✅ All 919 tests passing
  • ✅ Type checking and linting passing

Before & After

Before:

$ workato workspace
Traceback (most recent call last):
  File "/Users/.../workato", line 12, in <module>
    sys.exit(cli())
  [... 30 lines of Python stack trace ...]
  File ".../containers.py", line 40, in create_profile_aware_workato_config
    raise ValueError(
        ...<2 lines>...
    )
ValueError: Could not resolve API credentials. Please check your profile configuration or run 'workato init' to set up authentication.

After:

$ workato workspace
❌ ValueError
   Could not resolve API credentials. Please check your profile configuration or run 'workato init' to set up authentication.

Testing

  • All unit tests pass (62 tests for exception handler, 919 total)
  • Manually tested with commands before workato init
  • Verified both table and JSON output modes

Closes DEVP-424

🤖 Generated with Claude Code

…ly messages

Implements a comprehensive exception handling decorator that catches errors
during command initialization and dependency injection, replacing raw Python
stack traces with clear, actionable error messages.

Changes:
- Add handle_cli_exceptions decorator to catch all exceptions before they
  become stack traces
- Provide specific handling for known error types (network, timeout, SSL,
  server disconnect errors)
- Generic catch-all handler for unexpected exceptions shows error type and
  message without stack trace
- Apply decorator to 11 commands that use dependency injection
- Add 16 comprehensive unit tests covering sync/async functions and all
  error types

Key design principles:
- Conservative approach: only apply specific handlers when confident about
  error type
- Generic fallback ensures NO stack traces are ever shown to users
- Error messages are clean without redundant advice (original error messages
  already contain helpful context)

Before:
  $ workato workspace
  Traceback (most recent call last):
    [30 lines of Python stack trace...]
  ValueError: Could not resolve API credentials...

After:
  $ workato workspace
  ❌ ValueError
     Could not resolve API credentials. Please check your profile
     configuration or run 'workato init' to set up authentication.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Oct 29, 2025

Coverage

Coverage Report
FileStmtsMissCoverMissing
__init__.py68494%10–11, 69–70
cli
   __init__.py52394%49, 51, 60
   containers.py270100% 
cli/commands
   __init__.py00100% 
   api_clients.py291996%27, 302, 448–449, 483, 493, 584, 615, 623
   api_collections.py257398%28, 183, 347
   assets.py47295%55–56
   connections.py526599%589, 591, 597, 635, 986
   data_tables.py165596%31, 253, 267, 321–322
   guide.py166199%106
   init.py900100% 
   profiles.py2030100% 
   properties.py97198%21
   pull.py172298%193–194
   workspace.py39294%61, 71
cli/commands/connectors
   __init__.py00100% 
   command.py90297%110, 159
   connector_manager.py203498%176, 292, 300–301
cli/commands/projects
   __init__.py00100% 
   command.py2721096%359–362, 373, 439–441, 491, 495
   project_manager.py166795%48, 66, 263–264, 276, 317, 325
cli/commands/push
   __init__.py00100% 
   command.py133496%109, 112, 230, 308
cli/commands/recipes
   __init__.py00100% 
   command.py427997%117, 133–134, 272–275, 403, 709
   validator.py7062097%174, 883, 1136, 1223, 1246, 1279, 1281–1282, 1359–1361, 1457–1458, 1517–1518, 1707–1708, 1736–1738
cli/utils
   __init__.py30100% 
   exception_handler.py3042591%134–135, 140–141, 143–144, 170–171, 177, 220, 267, 294, 321, 378, 413, 472, 474–475, 480–481, 483–487
   gitignore.py140100% 
   ignore_patterns.py230100% 
   spinner.py430100% 
   version_checker.py135695%24, 26, 33–34, 72, 102
cli/utils/config
   __init__.py50100% 
   manager.py4671995%125, 136–138, 141, 154, 204–205, 357, 438–439, 478, 692, 835–836, 850–851, 912, 971
   models.py330100% 
   profiles.py3091096%93, 189–190, 193, 228–230, 255–257
   workspace.py680100% 
TOTAL560115397% 

The package was renamed from workato_platform to workato_platform_cli in
commit e7d7a23, but the .openapi-generator-ignore file wasn't updated.

This caused the OpenAPI generator pre-commit hook to overwrite the
workato_platform_cli/__init__.py file (which contains the Workato wrapper
class) with an empty template on every commit.

Fixed by updating the ignore pattern to match the new package name.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@hovu96 hovu96 requested review from cmworkato and oalami October 29, 2025 14:50
@hovu96 hovu96 marked this pull request as draft October 29, 2025 14:52
@oalami oalami requested a review from Copilot October 30, 2025 20:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a new handle_cli_exceptions decorator to handle CLI initialization and network errors with user-friendly messages. The decorator is designed to catch errors that occur during dependency injection and CLI setup, such as network failures, SSL errors, and configuration issues.

  • Implements the handle_cli_exceptions decorator with support for both sync and async functions
  • Adds dedicated error handlers for network, timeout, SSL, and generic CLI errors with JSON output support
  • Applies the decorator to multiple CLI commands throughout the codebase

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

File Description
src/workato_platform_cli/cli/utils/exception_handler.py Adds handle_cli_exceptions decorator and helper functions for handling CLI-level errors
tests/unit/utils/test_exception_handler.py Comprehensive test suite for the new decorator covering sync/async functions, various error types, and JSON output
src/workato_platform_cli/cli/commands/*.py Applies the new decorator to various command functions (workspace, recipes, push, pull, properties, projects, profiles, data_tables, connectors, connections, api_clients)
src/.openapi-generator-ignore Updates ignored path from workato_platform/__init__.py to workato_platform_cli/__init__.py

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@oalami oalami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good except for the exiting with a non-zero status code and the copilot raised issue on TimeoutError vs asyncio.TimeoutError looks worth implementing for better type safety.

hovu96 and others added 5 commits October 31, 2025 09:06
Ensures consistent exception handling across all CLI commands to catch
network errors (DNS, SSL, connection failures) and display user-friendly
error messages instead of stack traces.

Adds the decorator to 19 commands across 9 files:
- projects/command.py: list_projects
- connections.py: update, get_oauth_url
- data_tables.py: create_data_table
- api_clients.py: create, create_key, refresh_secret, list_api_keys
- api_collections.py: create, list_collections, list_endpoints, enable_endpoint
- recipes/command.py: list_recipes, start, update_connection
- connectors/command.py: list_connectors
- init.py: init
- assets.py: export, import_assets

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Previously, all exception handlers returned None, causing the CLI to exit
with code 0 even when errors occurred. This broke CI/CD pipelines and shell
scripts that rely on exit codes to detect failures.

Changes:
- Exception decorators now raise SystemExit(1) after handling errors
- Helper functions remain pure display logic (format and print errors)
- Consistent behavior across both JSON and table output modes
- Applies to both @handle_api_exceptions and @handle_cli_exceptions
- Uses "from None" to suppress exception chaining for clean error output

This ensures commands like:
  workato projects list && echo "success" || echo "failed"
will correctly show "failed" when errors occur.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Updated exception handler tests to expect SystemExit(1) instead of returning None,
matching the new exit code behavior implemented in the previous commit.

Changes:
- Updated all 43 exception handler tests to use pytest.raises(SystemExit) assertions
- Modified @handle_cli_exceptions decorator to let click.Abort propagate through
  instead of catching it, allowing init command tests to work correctly
- All 919 tests now pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
In Python 3.11+, asyncio.TimeoutError is an alias for the built-in
TimeoutError. This change makes the function signature consistent with
what's being caught in the exception handlers.

This is a cosmetic fix with no functional impact.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@hovu96 hovu96 marked this pull request as ready for review October 31, 2025 08:51
@hovu96
Copy link
Contributor Author

hovu96 commented Oct 31, 2025

Looks good except for the exiting with a non-zero status code and the copilot raised issue on TimeoutError vs asyncio.TimeoutError looks worth implementing for better type safety.

Addressed both:

  1. Exiting with a non-zero status
  2. Type annotation inconsistency (just cosmetic: asyncio.TimeoutError → TimeoutError since it's an alias)

@hovu96 hovu96 requested a review from oalami October 31, 2025 08:53
Copy link
Collaborator

@oalami oalami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@oalami oalami merged commit 4810381 into main Oct 31, 2025
5 checks passed
@oalami oalami deleted the bugfix/cli-exceptions branch October 31, 2025 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants