Part of duplicate code analysis: #4842
Summary
internal/cmd/root.go's postRun() function manually calls five individual CloseXxxLogger() functions, duplicating the logic already consolidated in logger.CloseAllLoggers(). The initialisation counterpart (InitGatewayLoggers) is already called as a single function, but cleanup was not updated to match.
Duplication Details
Pattern: Manual logger close calls vs. CloseAllLoggers()
- Severity: Low-Medium
- Occurrences: 5 redundant calls (+ 1 existing consolidated function)
- Locations:
internal/cmd/root.go lines 134–138 (the duplicate)
internal/logger/global_helpers.go lines 151–167 (CloseAllLoggers — the canonical implementation)
Duplicated code in root.go:
func postRun(cmd *cobra.Command, args []string) {
// Close all loggers
logger.CloseMarkdownLogger()
logger.CloseJSONLLogger()
logger.CloseServerFileLogger()
logger.CloseToolsLogger()
logger.CloseGlobalLogger()
}
Existing consolidated function in global_helpers.go:
func CloseAllLoggers() error {
closers := []func() error{
CloseGlobalLogger,
CloseJSONLLogger,
CloseMarkdownLogger,
CloseToolsLogger,
CloseServerFileLogger,
}
// ... iterates and closes all, capturing first error
}
Note: the manual calls also ignore all errors from each closer, while CloseAllLoggers() returns the first error encountered.
Impact Analysis
- Maintainability: When a new logger is added to
InitGatewayLoggers() and CloseAllLoggers(), root.go's postRun() is silently left out-of-date — a maintenance trap. This already happened once: InitGatewayLoggers is called as a unit but the matching CloseAllLoggers was not used.
- Bug Risk: Medium — new loggers added to
CloseAllLoggers() won't be closed at shutdown unless root.go is also updated. Resource leak risk on shutdown.
- Error Handling:
postRun silently discards all close errors; CloseAllLoggers() surfaces the first one.
Refactoring Recommendations
- Replace 5 calls with
logger.CloseAllLoggers() (recommended, simple fix)
func postRun(cmd *cobra.Command, args []string) {
if err := logger.CloseAllLoggers(); err != nil {
log.Printf("Warning: error closing loggers: %v", err)
}
}
- File to change:
internal/cmd/root.go
- Estimated effort: < 15 minutes
- Benefits: single source of truth, error surfacing, auto-picks-up new loggers
Implementation Checklist
Parent Issue
See parent analysis report: #4842
Related to #4842
Generated by Duplicate Code Detector · ● 2.6M · ◷
Part of duplicate code analysis: #4842
Summary
internal/cmd/root.go'spostRun()function manually calls five individualCloseXxxLogger()functions, duplicating the logic already consolidated inlogger.CloseAllLoggers(). The initialisation counterpart (InitGatewayLoggers) is already called as a single function, but cleanup was not updated to match.Duplication Details
Pattern: Manual logger close calls vs.
CloseAllLoggers()internal/cmd/root.golines 134–138 (the duplicate)internal/logger/global_helpers.golines 151–167 (CloseAllLoggers— the canonical implementation)Duplicated code in
root.go:Existing consolidated function in
global_helpers.go:Note: the manual calls also ignore all errors from each closer, while
CloseAllLoggers()returns the first error encountered.Impact Analysis
InitGatewayLoggers()andCloseAllLoggers(),root.go'spostRun()is silently left out-of-date — a maintenance trap. This already happened once:InitGatewayLoggersis called as a unit but the matchingCloseAllLoggerswas not used.CloseAllLoggers()won't be closed at shutdown unlessroot.gois also updated. Resource leak risk on shutdown.postRunsilently discards all close errors;CloseAllLoggers()surfaces the first one.Refactoring Recommendations
logger.CloseAllLoggers()(recommended, simple fix)internal/cmd/root.goImplementation Checklist
postRun()withlogger.CloseAllLoggers()root.godiffers fromCloseAllLoggers)Parent Issue
See parent analysis report: #4842
Related to #4842