Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds runtime validation for Traefik template rendering to detect and handle templating failures gracefully. The changes ensure that missing template variables and invalid YAML structures are caught early and logged appropriately, rather than silently failing or producing broken configurations.
Changes:
- Added comprehensive error handling in template rendering with validation for unknown variables and invalid YAML
- Modified
makeAppConfigto returnnullon template failures instead of throwing, allowing graceful skipping of problematic hosts - Enhanced middleware error handling and logging with better type safety and documentation
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/traefik/user-data-substitution.test.ts | New comprehensive tests for userData template substitution scenarios |
| test/unit/traefik/template-validation.test.ts | New tests validating generated Traefik config structure and detecting unreplaced variables |
| test/unit/traefik/template-error-handling.test.ts | New tests for error handling when templates fail to render |
| src/backends/traefik/traefik.ts | Added try-catch wrapper in makeAppConfig to handle template errors and skip failed hosts |
| src/backends/traefik/templateParser.ts | Enhanced to throw errors on unknown variables and invalid YAML instead of warnings |
| src/api/middleware/logging.ts | Improved type safety for variadic arguments in response.end override |
| src/api/middleware/errorHandler.ts | Refactored error property extraction with better documentation |
| .github/workflows/ci.yml | Added build step to test job with TODO comment about duplication |
|
|
||
| log.info({ message: 'Initializing Traefik backend', data: { templateCount: templatePaths.length } }); | ||
|
|
||
There was a problem hiding this comment.
Trailing whitespace on line 116. This line should not have trailing spaces after the comment marker.
| # TODO: duplicate build step; consider removing from here and only keeping in build-and-push | ||
| # once we figure a better way to prevent build errors from reaching build-and-push job. |
There was a problem hiding this comment.
The TODO comment is indented at the same level as the step name, which is inconsistent with YAML indentation conventions. The comment should either be placed above the step (unindented or at the same level as the step) or removed as an inline comment within the step.
| # TODO: duplicate build step; consider removing from here and only keeping in build-and-push | |
| # once we figure a better way to prevent build errors from reaching build-and-push job. | |
| # TODO: duplicate build step; consider removing from here and only keeping in build-and-push | |
| # once we figure a better way to prevent build errors from reaching build-and-push job. |
| userData: { missing_var: 'value' }, | ||
| }; | ||
|
|
||
| const badEntry: HostEntry = { |
There was a problem hiding this comment.
Unused variable badEntry.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.