Skip to content

Improve error handling in SetControllerReference calls#172

Merged
chrisguidry merged 1 commit intomainfrom
improve-error-handling
Jun 12, 2025
Merged

Improve error handling in SetControllerReference calls#172
chrisguidry merged 1 commit intomainfrom
improve-error-handling

Conversation

@chrisguidry
Copy link
Collaborator

Summary

This PR addresses the error handling gaps identified in issue #161 by implementing proper error handling for SetControllerReference calls in the PrefectServer controller.

Problem

The helper functions prefectServerDeployment() and prefectServerService() were ignoring errors from SetControllerReference calls with TODO comments, which could lead to:

  • Resources not being properly owned by their controllers
  • Silent failures that are difficult to debug
  • Potential runtime panics when invalid schemes are provided

Solution

  • Modified function signatures to return errors
  • Added nil scheme validation to prevent panics before calling SetControllerReference
  • Implemented proper error propagation from helper functions to callers
  • Added comprehensive error handling tests to verify proper behavior

Changes

  • prefectServerDeployment() now returns (deployment, pvc, job, error) instead of (deployment, pvc, job)
  • prefectServerService() now returns (service, error) instead of service
  • All call sites updated to handle the new error return values
  • Added 4 new test cases specifically for error handling scenarios

Test Coverage

  • Baseline coverage: 56.3%
  • Final coverage: 56.3% (maintained)
  • All tests passing: ✅

Testing

The new tests verify that:

  1. Helper functions properly return errors when scheme is nil
  2. Error messages are descriptive and helpful
  3. Errors are properly propagated through the reconciliation chain
  4. Different configuration scenarios (ephemeral, SQLite, PostgreSQL) handle errors correctly

Fixes #161

Test plan

  • All existing tests continue to pass
  • New error handling tests validate proper error propagation
  • Linters pass without issues
  • Coverage remains stable

🤖 Generated with Claude Code

This commit addresses the error handling gaps identified in issue #161.

Changes:
- Modified prefectServerDeployment() to return error and handle SetControllerReference failures
- Modified prefectServerService() to return error and handle SetControllerReference failures
- Added nil scheme checks to prevent panics
- Updated all call sites to handle the new error return values
- Added comprehensive error handling tests to verify proper error propagation

Before this change, SetControllerReference errors were silently ignored with TODO comments,
which could lead to resources not being properly owned by their controllers.

Now these functions properly validate the scheme and propagate SetControllerReference errors
to the caller, enabling proper error handling and status reporting.

Test coverage remains at 56.3% with all tests passing.

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

Co-Authored-By: Claude <noreply@anthropic.com>
@chrisguidry chrisguidry merged commit 9e0914d into main Jun 12, 2025
3 checks passed
@chrisguidry chrisguidry deleted the improve-error-handling branch June 12, 2025 13:40
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.

Bug: Missing Error Handling for Prefect Server Service and Deployment

2 participants