Fix silent exception swallowing in LegacyDataMigrator.migrate_project_data #1283

New issue

Open

opened 2026-04-02 08:44:25 +00:00 by freemo · 0 comments

freemo commented

2026-04-02 08:44:25 +00:00

Owner

Metadata

Branch: bugfix/m4-legacy-migrator-silent-exception
Commit Message: fix(error-handling): log exceptions in LegacyDataMigrator.migrate_project_data instead of swallowing them
Milestone: v3.3.0
Parent Epic: #362

Background and Context

The migrate_project_data method in LegacyDataMigrator catches json.JSONDecodeError and OSError exceptions but silently discards them, returning only False. This violates the project's error-handling principle that exceptions should not be suppressed without meaningful handling. When a legacy data migration fails — due to a corrupt JSON file, a missing file, or a filesystem permission error — there is no diagnostic information available to help a developer or operator understand what went wrong.

Current Behavior

When json.JSONDecodeError or OSError is raised inside migrate_project_data, the exception is caught and discarded. The function returns False with no log output. The caller has no way to distinguish between "file not found", "permission denied", and "malformed JSON" — all produce the same silent False.

# src/cleveragents/infrastructure/database/legacy_migrator.py:241
        except (json.JSONDecodeError, OSError):
            # Handle JSON parsing errors, file system errors gracefully
            return False

Expected Behavior

The exception should be logged at ERROR level with the file path and exception message before returning False, so that operators and developers can diagnose migration failures without resorting to attaching a debugger.

        except (json.JSONDecodeError, OSError) as e:
            # Handle JSON parsing errors, file system errors gracefully
            _logger.error("Failed to migrate legacy project data: %s", e)
            return False

Acceptance Criteria

LegacyDataMigrator.migrate_project_data logs an ERROR-level message (including the exception details) whenever a json.JSONDecodeError or OSError is caught
The function still returns False after logging (no change to the return contract)
A Behave scenario verifies that the error is logged when a JSONDecodeError is triggered
A Behave scenario verifies that the error is logged when an OSError is triggered
No new exceptions are raised by the change (the graceful-return contract is preserved)

Supporting Information

File: src/cleveragents/infrastructure/database/legacy_migrator.py
Function: LegacyDataMigrator.migrate_project_data
Lines: 241–243
Related Epic: #362 (Security & Safety Hardening — explicit exception handling)
Severity: High — silent failures make migration issues completely invisible in production logs
Likelihood: Medium — data corruption and permission errors are realistic operational scenarios

Subtasks

Write a TDD issue-capture Behave scenario tagged @tdd_issue, @tdd_expected_fail that asserts an ERROR log entry is emitted when migrate_project_data encounters a JSONDecodeError
Write a TDD issue-capture Behave scenario tagged @tdd_issue, @tdd_expected_fail that asserts an ERROR log entry is emitted when migrate_project_data encounters an OSError
Bind the exception in the except clause: except (json.JSONDecodeError, OSError) as e:
Add _logger.error("Failed to migrate legacy project data: %s", e) before return False (lines 241–243 of legacy_migrator.py)
Remove the @tdd_expected_fail tags from both TDD scenarios once the fix is in place and verify they pass
Run nox -e typecheck to confirm no type regressions
Run nox -e unit_tests to confirm all Behave tests pass
Run nox -e coverage_report to confirm coverage remains ≥ 97%
Run nox (all default sessions) and fix any errors

Definition of Done

This issue is complete when:

Both @tdd_expected_fail TDD capture scenarios are committed first and demonstrate the missing log output
The except clause in migrate_project_data binds the exception and logs it at ERROR level before returning False
Both TDD scenarios pass without @tdd_expected_fail after the fix
No regressions in existing migration-related tests
All nox stages pass (nox -e lint, nox -e typecheck, nox -e unit_tests, nox -e integration_tests, nox -e coverage_report)
Coverage >= 97%
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

## Metadata - **Branch**: `bugfix/m4-legacy-migrator-silent-exception` - **Commit Message**: `fix(error-handling): log exceptions in LegacyDataMigrator.migrate_project_data instead of swallowing them` - **Milestone**: v3.3.0 - **Parent Epic**: #362 ## Background and Context The `migrate_project_data` method in `LegacyDataMigrator` catches `json.JSONDecodeError` and `OSError` exceptions but silently discards them, returning only `False`. This violates the project's error-handling principle that exceptions should not be suppressed without meaningful handling. When a legacy data migration fails — due to a corrupt JSON file, a missing file, or a filesystem permission error — there is no diagnostic information available to help a developer or operator understand what went wrong. ## Current Behavior When `json.JSONDecodeError` or `OSError` is raised inside `migrate_project_data`, the exception is caught and discarded. The function returns `False` with no log output. The caller has no way to distinguish between "file not found", "permission denied", and "malformed JSON" — all produce the same silent `False`. ```python # src/cleveragents/infrastructure/database/legacy_migrator.py:241 except (json.JSONDecodeError, OSError): # Handle JSON parsing errors, file system errors gracefully return False ``` ## Expected Behavior The exception should be logged at `ERROR` level with the file path and exception message before returning `False`, so that operators and developers can diagnose migration failures without resorting to attaching a debugger. ```python except (json.JSONDecodeError, OSError) as e: # Handle JSON parsing errors, file system errors gracefully _logger.error("Failed to migrate legacy project data: %s", e) return False ``` ## Acceptance Criteria - [ ] `LegacyDataMigrator.migrate_project_data` logs an `ERROR`-level message (including the exception details) whenever a `json.JSONDecodeError` or `OSError` is caught - [ ] The function still returns `False` after logging (no change to the return contract) - [ ] A Behave scenario verifies that the error is logged when a `JSONDecodeError` is triggered - [ ] A Behave scenario verifies that the error is logged when an `OSError` is triggered - [ ] No new exceptions are raised by the change (the graceful-return contract is preserved) ## Supporting Information - **File**: `src/cleveragents/infrastructure/database/legacy_migrator.py` - **Function**: `LegacyDataMigrator.migrate_project_data` - **Lines**: 241–243 - **Related Epic**: #362 (Security & Safety Hardening — explicit exception handling) - **Severity**: High — silent failures make migration issues completely invisible in production logs - **Likelihood**: Medium — data corruption and permission errors are realistic operational scenarios ## Subtasks - [ ] Write a TDD issue-capture Behave scenario tagged `@tdd_issue`, `@tdd_expected_fail` that asserts an `ERROR` log entry is emitted when `migrate_project_data` encounters a `JSONDecodeError` - [ ] Write a TDD issue-capture Behave scenario tagged `@tdd_issue`, `@tdd_expected_fail` that asserts an `ERROR` log entry is emitted when `migrate_project_data` encounters an `OSError` - [ ] Bind the exception in the `except` clause: `except (json.JSONDecodeError, OSError) as e:` - [ ] Add `_logger.error("Failed to migrate legacy project data: %s", e)` before `return False` (lines 241–243 of `legacy_migrator.py`) - [ ] Remove the `@tdd_expected_fail` tags from both TDD scenarios once the fix is in place and verify they pass - [ ] Run `nox -e typecheck` to confirm no type regressions - [ ] Run `nox -e unit_tests` to confirm all Behave tests pass - [ ] Run `nox -e coverage_report` to confirm coverage remains ≥ 97% - [ ] Run `nox` (all default sessions) and fix any errors ## Definition of Done This issue is complete when: - [ ] Both `@tdd_expected_fail` TDD capture scenarios are committed first and demonstrate the missing log output - [ ] The `except` clause in `migrate_project_data` binds the exception and logs it at `ERROR` level before returning `False` - [ ] Both TDD scenarios pass without `@tdd_expected_fail` after the fix - [ ] No regressions in existing migration-related tests - [ ] All nox stages pass (`nox -e lint`, `nox -e typecheck`, `nox -e unit_tests`, `nox -e integration_tests`, `nox -e coverage_report`) - [ ] Coverage >= 97% - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done.

freemo added the

labels

2026-04-02 08:44:51 +00:00

freemo added this to the v3.3.0 milestone

2026-04-02 08:45:03 +00:00

freemo referenced this issue