refactor: add configuration and error handling infrastructure#59
Closed
JanZachmann wants to merge 4 commits intoomnect:mainfrom
Closed
refactor: add configuration and error handling infrastructure#59JanZachmann wants to merge 4 commits intoomnect:mainfrom
JanZachmann wants to merge 4 commits intoomnect:mainfrom
Conversation
Replace hyper-util with reqwest for all HTTP/Unix socket communication, remove Arc wrappers, and eliminate the socket_client module to simplify the codebase. Changes: - Remove Arc wrappers from Api struct fields (OmnectDeviceServiceClient and SingleSignOnProvider are already cheap to clone via internal pools) - Replace hyper-util Client with reqwest for Unix socket communication using reqwest 0.12.23's native unix_socket() support - Remove socket_client.rs module entirely (~109 lines removed) - Inline HTTP request logic directly in OmnectDeviceServiceClient with cleaner helper methods: get(), post(), post_json() - Convert keycloak_client from blocking to async reqwest - Update certificate.rs to use reqwest::Client directly Dependencies: - Remove: http-body-util, hyper, hyperlocal, hyper-util - Update: reqwest from 0.12 to 0.12.23 (minimum for Unix socket support) - Remove: blocking feature from reqwest (no longer needed) Benefits: - Single HTTP client library throughout codebase - Simpler, more maintainable code with direct reqwest usage - Better error messages and logging - Removed unnecessary abstraction layers - Zero runtime overhead (no Arc, reqwest uses internal Arc) Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>
This commit implements comprehensive refactorings across the codebase:
Performance Optimizations:
- Cache Keycloak HTTP client using OnceLock to avoid repeated instantiation
- Consolidate HTTP client usage patterns
Code Quality Improvements:
- Add HTTP response handling helper to reduce duplication
- Consolidate path macros for consistency
- Add API error handling helper with standardized logging
- Use endpoint constants instead of string literals
- Remove unnecessary .map(|_| ()) patterns
- Standardize error logging format to {e:#}
- Standardize all error messages to use "failed to..." format
Maintainability:
- Replace magic string "omnect-ui" with env!("CARGO_PKG_NAME")
- Improve string interpolation to use direct variable references
- Optimize imports (remove wildcard imports, use explicit imports)
- Fix inconsistent crate imports in main.rs
Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>
Problems addressed: - Inconsistent cleanup across different shutdown paths (ctrl-c, SIGTERM, server crash, centrifugo crash) - Race conditions between service client shutdown and centrifugo termination - Mixed error handling (panics vs logging) - No guaranteed cleanup order Solution: - Unified shutdown sequence that runs regardless of exit reason - Consistent cleanup order: service client → server → centrifugo - Graceful error handling throughout (no panics) - Clear debug logging at each step Benefits: - Predictable shutdown behavior in all scenarios - Proper resource cleanup guaranteed - Better observability with consistent logging - Safer error handling Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>
This was referenced Oct 26, 2025
This commit introduces new infrastructure modules without changing existing code, preparing for gradual migration in future PRs. New Modules: - config.rs: Centralized configuration management with validation - AppConfig struct with all environment variable configuration - Explicit validation at startup instead of lazy initialization - IoT Edge configuration detection - Path configuration helpers - errors.rs: Structured error types with HTTP status mapping - OmnectUiError enum with specific error categories - Automatic HTTP status code mapping via ResponseError trait - ErrorContext trait for adding typed error context - Better error diagnostics for API responses Benefits: - Foundation for removing global OnceLock state in future PRs - Type-safe configuration with clear documentation - Structured error handling improves debugging and API responses - No breaking changes to existing code Future Work: - PR #2: Consolidate HTTP clients using config - PR #3: Migrate authentication and testing to use new infrastructure
JanZachmann
force-pushed
the
refactor-config-and-errors
branch
from
1ecfb37 to
b461c46
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces foundational infrastructure for configuration management and error handling.
Note: This PR is based on the
optimize-performance-and-safetybranch and includes those changes as well.Changes
New Module:
config.rs(368 lines)AppConfig: Centralized configuration struct with validationBenefits:
New Module:
errors.rsOmnectUiError: Structured error enum with HTTP status mapping:Authentication→ 401 UnauthorizedServiceUnavailable→ 503 Service UnavailableCertificate→ 400 Bad RequestConfiguration→ 500 Internal Server ErrorValidationError→ 400 Bad RequestInternal→ 500 Internal Server ErrorErrorContexttrait: Add typed error contextTesting
✅ All tests pass (13 tests)
✅ cargo check passes
✅ cargo clippy passes
✅ cargo fmt applied
Impact
🤖 Generated with Claude Code