Skip to content

Enforce Cognee error constructor contracts#4138

Open
silasbrookshaha wants to merge 1 commit into
topoteretes:devfrom
silasbrookshaha:fix-cognee-error-contracts
Open

Enforce Cognee error constructor contracts#4138
silasbrookshaha wants to merge 1 commit into
topoteretes:devfrom
silasbrookshaha:fix-cognee-error-contracts

Conversation

@silasbrookshaha

Copy link
Copy Markdown
Contributor

Description

Fixes #4131.

This fixes Cognee error subclasses that manually set message, name, and status_code without calling the base exception initializer. Those classes now preserve Exception.args and the existing string/status behavior. For the two database exceptions that were intentionally non-logging paths, the fix keeps log=False while still running the base initializer.

I also added a reflection-based contract test that loads Cognee exception modules, walks the full CogneeApiError subclass tree, instantiates every class with representative arguments, and verifies:

  • str(exc) includes the stored message
  • exc.args is populated with that message
  • raise exc from cause preserves __cause__

Acceptance Criteria

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Code refactoring
  • Other (please specify):

Screenshots

N/A. Local verification passed:

  • /private/tmp/cognee-contract-venv/bin/python -m pytest cognee/tests/unit/exceptions/test_cognee_error_contract.py -q
  • /private/tmp/cognee-contract-venv/bin/python -m pytest cognee/tests/unit/modules/pipelines -q
  • /private/tmp/cognee-contract-venv/bin/python -m py_compile cognee/infrastructure/databases/exceptions/exceptions.py cognee/modules/pipelines/exceptions/tasks.py cognee/tests/unit/exceptions/test_cognee_error_contract.py
  • git diff --cached --check

Pre-submission Checklist

  • I have tested my changes thoroughly before submitting this PR (See CONTRIBUTING.md)
  • This PR contains minimal changes necessary to address the issue/feature
  • My code follows the project's coding standards and style guidelines
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if applicable)
  • All new and relevant existing tests pass
  • I have searched existing PRs to ensure this change hasn't been submitted already
  • I have linked any relevant issues in the description
  • My commits have clear and descriptive messages

DCO Affirmation

I affirm that all code in every commit of this pull request conforms to the terms of the Topoteretes Developer Certificate of Origin.

@github-actions

Copy link
Copy Markdown
Contributor

Hello @silasbrookshaha, thank you for submitting a PR! We will respond as soon as possible.

rshkarin added a commit that referenced this pull request Jul 21, 2026
…-240]

EntityNotFoundError and NodesetFilterNotSupportedError
(infrastructure/databases/exceptions) and WrongTaskTypeError
(modules/pipelines/exceptions/tasks) overrode __init__ but never called
super().__init__(), leaving Exception.args empty and bypassing centralized
logging/chaining (gh #3749).

The two database exceptions are raised in routine control flow (caught in
user resolution, migrations, pruning, graph projection), so they pass
log=False — restoring args and chaining without emitting ERROR logs for
expected conditions. WrongTaskTypeError is a genuine programming error and
keeps default logging.

Adds cognee/tests/unit/exceptions/test_cognee_error_contract.py with two
guards that prevent the whole bug class from returning:
- a static AST sweep of every .py in the package (import-free, so it also
  covers subclasses outside */exceptions/ and behind optional extras);
- a runtime test that instantiates every importable subclass and asserts
  args populated, str() non-empty, and __cause__ preserved.

Builds on the approach in #4138 by @silasbrookshaha, extending coverage to
the third affected class and to subclasses the original importer missed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rshkarin added a commit that referenced this pull request Jul 21, 2026
…-240]

EntityNotFoundError and NodesetFilterNotSupportedError
(infrastructure/databases/exceptions) and WrongTaskTypeError
(modules/pipelines/exceptions/tasks) overrode __init__ but never called
super().__init__(), leaving Exception.args empty and bypassing centralized
logging/chaining (gh #3749).

The two database exceptions are raised in routine control flow (caught in
user resolution, migrations, pruning, graph projection), so they pass
log=False — restoring args and chaining without emitting ERROR logs for
expected conditions. WrongTaskTypeError is a genuine programming error and
keeps default logging.

Adds cognee/tests/unit/exceptions/test_cognee_error_contract.py with two
guards that prevent the whole bug class from returning:
- a static AST sweep of every .py in the package (import-free, so it also
  covers subclasses outside */exceptions/ and behind optional extras);
- a runtime test that instantiates every importable subclass and asserts
  args populated, str() non-empty, and __cause__ preserved.

Builds on the approach in #4138 by @silasbrookshaha, extending coverage to
the third affected class and to subclasses the original importer missed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rshkarin added a commit that referenced this pull request Jul 22, 2026
…-240]

EntityNotFoundError and NodesetFilterNotSupportedError
(infrastructure/databases/exceptions) and WrongTaskTypeError
(modules/pipelines/exceptions/tasks) overrode __init__ but never called
super().__init__(), leaving Exception.args empty and bypassing centralized
logging/chaining (gh #3749).

The two database exceptions are raised in routine control flow (caught in
user resolution, migrations, pruning, graph projection), so they pass
log=False — restoring args and chaining without emitting ERROR logs for
expected conditions. WrongTaskTypeError is a genuine programming error and
keeps default logging.

Adds cognee/tests/unit/exceptions/test_cognee_error_contract.py with two
guards that prevent the whole bug class from returning:
- a static AST sweep of every .py in the package (import-free, so it also
  covers subclasses outside */exceptions/ and behind optional extras);
- a runtime test that instantiates every importable subclass and asserts
  args populated, str() non-empty, and __cause__ preserved.

Builds on the approach in #4138 by @silasbrookshaha, extending coverage to
the third affected class and to subclasses the original importer missed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rshkarin added a commit that referenced this pull request Jul 22, 2026
…-240]

EntityNotFoundError and NodesetFilterNotSupportedError
(infrastructure/databases/exceptions) and WrongTaskTypeError
(modules/pipelines/exceptions/tasks) overrode __init__ but never called
super().__init__(), leaving Exception.args empty and bypassing centralized
logging/chaining (gh #3749).

The two database exceptions are raised in routine control flow (caught in
user resolution, migrations, pruning, graph projection), so they pass
log=False — restoring args and chaining without emitting ERROR logs for
expected conditions. WrongTaskTypeError is a genuine programming error and
keeps default logging.

Adds cognee/tests/unit/exceptions/test_cognee_error_contract.py with two
guards that prevent the whole bug class from returning:
- a static AST sweep of every .py in the package (import-free, so it also
  covers subclasses outside */exceptions/ and behind optional extras);
- a runtime test that instantiates every importable subclass and asserts
  args populated, str() non-empty, and __cause__ preserved.

Builds on the approach in #4138 by @silasbrookshaha, extending coverage to
the third affected class and to subclasses the original importer missed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rshkarin added a commit that referenced this pull request Jul 23, 2026
…-240]

EntityNotFoundError and NodesetFilterNotSupportedError
(infrastructure/databases/exceptions) and WrongTaskTypeError
(modules/pipelines/exceptions/tasks) overrode __init__ but never called
super().__init__(), leaving Exception.args empty and bypassing centralized
logging/chaining (gh #3749).

The two database exceptions are raised in routine control flow (caught in
user resolution, migrations, pruning, graph projection), so they pass
log=False — restoring args and chaining without emitting ERROR logs for
expected conditions. WrongTaskTypeError is a genuine programming error and
keeps default logging.

Adds cognee/tests/unit/exceptions/test_cognee_error_contract.py with two
guards that prevent the whole bug class from returning:
- a static AST sweep of every .py in the package (import-free, so it also
  covers subclasses outside */exceptions/ and behind optional extras);
- a runtime test that instantiates every importable subclass and asserts
  args populated, str() non-empty, and __cause__ preserved.

Builds on the approach in #4138 by @silasbrookshaha, extending coverage to
the third affected class and to subclasses the original importer missed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rshkarin added a commit that referenced this pull request Jul 23, 2026
…-240]

EntityNotFoundError and NodesetFilterNotSupportedError
(infrastructure/databases/exceptions) and WrongTaskTypeError
(modules/pipelines/exceptions/tasks) overrode __init__ but never called
super().__init__(), leaving Exception.args empty and bypassing centralized
logging/chaining (gh #3749).

The two database exceptions are raised in routine control flow (caught in
user resolution, migrations, pruning, graph projection), so they pass
log=False — restoring args and chaining without emitting ERROR logs for
expected conditions. WrongTaskTypeError is a genuine programming error and
keeps default logging.

Adds cognee/tests/unit/exceptions/test_cognee_error_contract.py with two
guards that prevent the whole bug class from returning:
- a static AST sweep of every .py in the package (import-free, so it also
  covers subclasses outside */exceptions/ and behind optional extras);
- a runtime test that instantiates every importable subclass and asserts
  args populated, str() non-empty, and __cause__ preserved.

Builds on the approach in #4138 by @silasbrookshaha, extending coverage to
the third affected class and to subclasses the original importer missed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Vasilije1990 added a commit that referenced this pull request Jul 25, 2026
…-240] (#4151)

## Summary

Fixes GitHub #3749 (Linear **SDK-240**, related to **SDK-217**). Three
`CogneeApiError` subclasses overrode `__init__` but never called
`super().__init__()`, so `Exception.args` stayed empty `()` — breaking
`repr()`, exception chaining, and the centralized logging in
`CogneeApiError.__init__`.

An AST sweep of every family subclass in the repo confirms **exactly
three** offenders on `dev`:

| Class | File | In #3749? |
|-------|------|-----------|
| `EntityNotFoundError` |
`infrastructure/databases/exceptions/exceptions.py` | ✅ named |
| `NodesetFilterNotSupportedError` |
`infrastructure/databases/exceptions/exceptions.py` | ✅ named |
| `WrongTaskTypeError` | `modules/pipelines/exceptions/tasks.py` | ❌
same bug, not named in the issue |

## The `log=False` nuance

`EntityNotFoundError` is raised in **routine control flow** and caught
in ~10 places (`cli/user_resolution.py`, `migrations/runner.py`,
`data/deletion/prune_system.py`, `brute_force_triplet_search.py`,
`CogneeGraph` empty-graph/neighborhood cases,
`create_authorized_dataset.py`, …). The original `# TODO … we shouldn't
log error` comment was intentional. So the two database exceptions pass
`super().__init__(..., log=False)` — restoring `args`/chaining
**without** flooding logs with ERROR lines for expected conditions
(matching the existing `UnsupportedProvenanceCapability` idiom).
`WrongTaskTypeError` is a genuine programming error and keeps default
logging.

## Regression guard

Adds `cognee/tests/unit/exceptions/test_cognee_error_contract.py` with
two complementary checks:

1. **Static AST sweep** of every `.py` in the package — import-free, so
it also covers `CogneeApiError` subclasses that live **outside**
`*/exceptions/` (e.g. `modules/retrieval/code_retriever.py`,
`tasks/web_scraper/ssrf_protection.py`, `tasks/code_graph/*`) and
modules behind optional extras.
2. **Runtime contract** — instantiates every importable subclass and
asserts `args` is populated, `str()` is non-empty, and `raise … from
cause` preserves `__cause__`.

Verified both guards **fail** on the pre-fix code and **pass** after;
existing `test_user_resolution.py` still green.

## Credit

Builds on the approach in #4138 by @silasbrookshaha, extending it to the
third affected class (`WrongTaskTypeError`) and closing a coverage gap
in the original reflection test's importer (subclasses outside
`*/exceptions/`). Supersedes the partial fixes in #3750, #3803, #3837
(the latter two drop `log=False`, reintroducing ERROR-log noise for
routinely-caught `EntityNotFoundError`).

## Checklist
- [x] Bug fix (non-breaking)
- [x] Tests added; verified they fail on the buggy code
- [x] `ruff format` / `ruff check` clean

I have read the CLA Document and I hereby sign the CLA.
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.

1 participant