Skip to content

fix: narrow exception in extension registration fallback from Exception to specific types - #3919

Open
Quratulain-bilal wants to merge 2 commits into
github:mainfrom
Quratulain-bilal:fix/presets-registration-narrow-exception
Open

fix: narrow exception in extension registration fallback from Exception to specific types#3919
Quratulain-bilal wants to merge 2 commits into
github:mainfrom
Quratulain-bilal:fix/presets-registration-narrow-exception

Conversation

@Quratulain-bilal

Copy link
Copy Markdown
Contributor

Problem

Bare except Exception silently swallows all errors in extension registration.

Fix

Narrow to (TypeError, ValueError, KeyError) which are the realistic failure modes.

Testing

  • Verified extension registration works correctly
  • Verified fallback to generic path-based registration works

…on to specific types

Bare 'except Exception' silently swallows all errors. Narrow to
(TypeError, ValueError, KeyError) which are the realistic failure
modes in extension registration.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Narrows extension-registration fallback exceptions to avoid swallowing unexpected errors.

Changes:

  • Replaces broad Exception handling with specific exception types.
  • However, omits the expected ValidationError from invalid manifests.
Show a summary per file
File Description
src/specify_cli/presets/__init__.py Narrows extension registration fallback exceptions.

Review details

Tip

Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/specify_cli/presets/__init__.py Outdated
record_written(written)
registered = True
except Exception:
except (TypeError, ValueError, KeyError):
…on to specific types

Catch (TypeError, ValueError, KeyError, ValidationError) instead of
bare Exception. ExtensionManifest raises ValidationError (not ValueError)
for malformed/unreadable manifests, so it must be caught to preserve the
fallback to generic path-based registration.

Add regression test verifying ValidationError during manifest load is
silently skipped.

Co-authored-by: GitHub Copilot (model: mimo-v2.5-free, supervised)
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