Skip to content

Refactor binoculars internal library to eliminate os.Exit usage#4858

Open
itsfarhan wants to merge 1 commit intoarmadaproject:masterfrom
itsfarhan:pr-issue-4779
Open

Refactor binoculars internal library to eliminate os.Exit usage#4858
itsfarhan wants to merge 1 commit intoarmadaproject:masterfrom
itsfarhan:pr-issue-4779

Conversation

@itsfarhan
Copy link
Copy Markdown

What type of PR is this?

Refactoring internal library - binoculars

What this PR does / why we need it

Refactored the binoculars internal library to remove process terminating os.Exit calls, replacing them with returns as mentioned in the issue.
By changing the code to return an error, we give control back to cmd/binoculars/main.go (the main entry point). The main function will handle fatal errors and exit gracefully.

Which issue(s) this PR fixes

Fixes #4779

Special notes for your reviewer

Tested in local with these commands

go build ./cmd/binoculars/...
golangci-lint run ./internal/binoculars/... ./cmd/binoculars/...

Screenshot:
Screenshot from 2026-04-22 21-30-20

Signed-off-by: itsfarhan <itsmefarhan09@gmail.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 22, 2026

Greptile Summary

This PR refactors internal/binoculars/server.go to eliminate direct os.Exit calls from library code, replacing them with idiomatic Go error returns. The cmd/binoculars/main.go call-site is updated to handle the new error return with log.Fatalf, correctly keeping process-exit decisions at the command layer.

Confidence Score: 5/5

Safe to merge — straightforward refactoring with no behaviour change for callers.

Both changed files are correct: the library now propagates errors instead of exiting, the main function handles them at the right boundary, and no existing tests break. All findings are at most P2.

No files require special attention.

Important Files Changed

Filename Overview
internal/binoculars/server.go Refactored StartUp to return (func(), *sync.WaitGroup, error) instead of calling os.Exit; replaces two os.Exit(-1) calls with wrapped error returns and removes the log/os imports.
cmd/binoculars/main.go Updated call-site to capture the new error return from StartUp and handle it with log.Fatalf, preserving process-exit semantics at the correct boundary (cmd layer).

Sequence Diagram

sequenceDiagram
    participant M as cmd/binoculars/main.go
    participant S as internal/binoculars/server.go (StartUp)
    participant K as cluster.NewKubernetesClientProvider
    participant A as auth.ConfigureAuth
    participant G as grpcCommon.Listen

    M->>S: StartUp(&config)
    S->>K: NewKubernetesClientProvider(...)
    alt error
        K-->>S: err
        S-->>M: nil, nil, error
        M->>M: log.Fatalf (process exits)
    else success
        K-->>S: kubernetesClientProvider
    end
    S->>A: ConfigureAuth(config.Auth)
    alt error
        A-->>S: err
        S-->>M: nil, nil, error
        M->>M: log.Fatalf (process exits)
    else success
        A-->>S: authServices
    end
    S->>G: Listen(config.GrpcPort, grpcServer, wg)
    S-->>M: grpcServer.GracefulStop, wg, nil
    M->>M: wg.Wait()
Loading

Reviews (1): Last reviewed commit: "refactored binoculars internal lib to re..." | Re-trigger Greptile

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.

Replace os.Exit in binoculars library with error return

1 participant