Codecov Report

❌ Patch coverage is 0% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 7.17%. Comparing base (60ad0bd) to head (b2b7127).
⚠️ Report is 87 commits behind head on main.

@@            Coverage Diff            @@
##             main    #678      +/-   ##
=========================================
- Coverage   10.99%   7.17%   -3.82%     
=========================================
  Files         173     260      +87     
  Lines        8671   12850    +4179     
=========================================
- Hits          953     922      -31     
- Misses       7612   11820    +4208     
- Partials      106     108       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CC: @bupd @qcserestipy
While testing out this bug, I found out that currently our logrus logger used for logging errors get configured gets configured once we reach the followlogs function ( reference here ) because of which any validation we do before that (such as the one done here) doesn't get logged. Sample output :

So even though we are successfully catching the error, the CLI doesnt show it, which I think isnt ideal. What do you think?

There are 2 options in my opinion to fix this :

1. We configure the logger during init in the main.go

• that way it will fix this for all the commands (I suspect this issue is not only for this command).

2. If we only want to configure the logger in the implementation functions ( eg: followLogs) (like we are doing right now), we can use :

    fmt.Fprintf(os.Stderr, "Error: invalid refresh interval: %v\n", err)
    os.Exit(1)

during the CLI input validation, we will have to make this change in all the CLI commands.

I looked online for what is better approach, and although they say that the first option make the CLI slighly slow(because this happens at init), I would recommend that only, because I dont think it is that significant, and it is cleaner implementation.
What do you suggest?

Great investigation! that's an important UX issue - the error gets swallowed because the logger isn't configured yet.

I think Option 2 is the better approach for a few reasons:

1. Keeps validation close to usage - The error happens in followLogs(), so validating there makes the code more maintainable and easier to understand.

2. More flexible - Different commands might have different validation needs. Centralizing in main.go could make that harder to manage.

3. Cleaner separation of concerns - main.go handles initialization and routing, while command implementations handle their own validation.

For the implementation, using fmt.Fprintf(os.Stderr, ...) + os.Exit(1) is perfect because:

• It prints directly to stderr (doesn't need logger)
• It's clear and immediate
• It's consistent with how CLI tools typically handle validation errors

One small suggestion: You could wrap this in a helper function to keep it DRY across commands:

func validateRefreshInterval(interval time.Duration) {
    if interval <= 0 {
        fmt.Fprintf(os.Stderr, "Error: invalid refresh interval: %v\n", interval)
        os.Exit(1)
    }
}

Then just call validateRefreshInterval(interval) in each command that needs it.
what you say @NucleoFusion ?!

Why not keep it simple? And return an error?
Using the RunE field instead of Run in Cobra?

Why not keep it simple? And return an error? Using the RunE field instead of Run in Cobra?

sounds cool and maintainable to me

I'd also suggest @NucleoFusion's point. We can simply return an error and make it a RunE command. I think this is something we should do with most commands in the long run anyway.

Yeah that sounds good. Will do that.
I think we should make this change across all the cmd functions as well. Does this sound good?

@adityachopra29 that is a seperate PR. Already being worked upon (if not, feel free to have a look)

Oh okay.....
This PR is ready to be merged, PTAL @bupd @qcserestipy