Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kpm needs Debug Mode to show and record more detailed logs #366

Open
zong-zhe opened this issue Jun 20, 2024 · 5 comments
Open

kpm needs Debug Mode to show and record more detailed logs #366

zong-zhe opened this issue Jun 20, 2024 · 5 comments
Assignees
Labels
feature request Feature request. help wanted Extra attention is needed

Comments

@zong-zhe
Copy link
Contributor

Feature Request

Is your feature request related to a problem? Please describe:

In some cases, the user needs to be able to see some more detailed log content to help the user understand what happened.

Describe the feature you'd like:

Add environment variables and CLI flag to control the Debug Mode on or off, and save the logs generated during kpm daily work in the log file in the ~/.kcl/kpm directory.

Describe alternatives you've considered:

Teachability, Documentation, Adoption, Migration Strategy:

@zong-zhe zong-zhe added help wanted Extra attention is needed feature request Feature request. labels Jun 20, 2024
@vinayakjaas
Copy link

Hey @zong-zhe, could you please assign this issue to me? I would like to work on it and come up with a solution.

@zong-zhe
Copy link
Contributor Author

Hi @vinayakjaas 😄,

Welcome!

I have assigned this task to you, there are some more details.

This task mainly includes three parts,

  1. kpm's current error management is a bit confusing, you may need to investigate some go programming error management common patterns, and then design a simple and lightweight error throwing mechanism for kpm, and then.
  2. The second step is to add a log module to kpm, reuse some common log tripartite libraries in go language, and record this log during the program running.
  3. Finally, add switches to set the debug mode of the logging system, log redirection, and so on.

You can begin with a paperwork for Part 1, just update it into this issue, we can discuss and finally decide how to implement part one.

@vinayakjaas
Copy link

Hey @zong-zhe I go through by various blogs and documentation to find out common patterns for error management in go programming and I come to following conclusion

  • The most common approach where functions return error values alongside the expected results.
  • Defining custom error types to provide more context.
  • Using fmt.Errorf with %w or libraries like pkg/errors for wrapping errors to provide context while preserving the original error.
  • Libraries like pkg/errors or go-errors/errors that provide more sophisticated error handling mechanisms.

By going through all reading part I plan approach to solve issue (part 1).This approach will enhance the clarity, consistency, and maintainability of the error management system in kpm.

  • We will introduce a custom error type, KpmError, which will encapsulate error messages and underlying errors. This custom error type will provide a clear and consistent way to handle various error scenarios within kpm.
  • Utility functions will be created to wrap errors with additional context and ensure they conform to the KpmError type. These functions will be used throughout the application to generate and manage errors consistently.
  • A centralized error handling mechanism will be implemented to log errors and provide meaningful messages to the users. This will involve creating a function that logs errors and prints user-friendly messages.

Benefits

  • Clear categorization of errors.
  • Easier to manage and track specific error types.
  • Simplifies error creation and handling.
  • Single point of error management.
  • Ensures all errors are logged properly.

I am happy to listen you feedback and suggestions. So I move further accordingly
Thank You

Reference :-
https://earthly.dev/blog/golang-errors/
https://blog.logrocket.com/error-handling-golang-best-practices/

@zong-zhe
Copy link
Contributor Author

Hi @vinayakjaas 😄,

Good job!I think the overall is in the right direction, next, you may need to provide some pseudo-codes, some simple examples, or demo your idea in a PR to show more details.

For the three points, show a simple demo for them in a go file and same examples about how to use. A simple demo to help us understand better.

  • We will introduce a custom error type, KpmError, which will encapsulate error messages and underlying errors. This custom error type will provide a clear and consistent way to handle various error scenarios within kpm.
  • Utility functions will be created to wrap errors with additional context and ensure they conform to the KpmError type. These functions will be used throughout the application to generate and manage errors consistently.
  • A centralized error handling mechanism will be implemented to log errors and provide meaningful messages to the users. This will involve creating a function that logs errors and prints user-friendly messages.

And one more thing I want to add about the logging system is that there may be some more non-error details in the logging system, which are not errors, but simply identifying what kpm is currently doing.

@vinayakjaas
Copy link

vinayakjaas commented Jul 24, 2024

Hey @zong-zhe I open Pull Request which cover Error Handling part in proper way. The following changes have been made:

Updated errors.go

  • Custom Error Type: KpmError provides a consistent structure for error messages with context.
  • Utility Function: NewKpmError creates a new KpmError instance with a message, an underlying error, and a context string.
  • Error Variables: Updated all error variables to use the NewKpmError function with appropriate messages and contexts.

reporter.go

  • Import Custom Errors: Added import for the custom errors package.
  • LogError Function: Added LogError function for centralized error logging using logrus.
  • WrapError Function: Added WrapError function to wrap an error with additional context and log it using logrus.

kpm.go

  • Replace reporter.Fatal with reporter.WrapError:
  • reporter.Fatal(reporter.WrapError("context message", err)) is used to wrap the error with a context message and then log it as a fatal error.
  • Added contextual messages to the errors to provide more details about where the error occurred.

Demo

Simulate an Error in client.NewKpmClient():
Modify the client.NewKpmClient function temporarily to return an error. This will simulate a failure in client initialization.

func NewKpmClient() (*KpmClient, error) {
    return nil, goerr.New("simulated client creation error")
}

Here I share two screenshots :-
pr1
Before changes in codebase
PR2
After changes in codebase
I not able to test for some other error so I request please review my PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request. help wanted Extra attention is needed
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants