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

Add config log levels #70

Closed
wants to merge 1 commit into from

Conversation

rennanboni
Copy link

Hi,

Would be possible to add log levels for custom output
I add the levels configurations log, debug, error, assert, trace, warn, I know is a bit too much for what plugin really needs and use, we could cut to log, debug and error if you wish

But I wanted level error added so I could control in my app

What you think?

@hg-pyun hg-pyun self-requested a review October 8, 2020 05:32
@hg-pyun hg-pyun added the enhancement New feature or request label Oct 8, 2020
@hg-pyun
Copy link
Owner

hg-pyun commented Oct 8, 2020

@rennanboni
Hello. Thank you for your good opinion.
Because loggers cannot predict how they will be used, they are abstracted and provided. I think it would be better to pass the parameters so that they can be distinguished within the logger function rather than by level. I think we'd better go over that feature a little bit more.

@rennanboni
Copy link
Author

Hi @hg-pyun ,

Sorry for the delay, its been a busy month
Could you tell more about how the config would like

What I did understand is the config would look like this

{
  "logger": {
    "onSuccess": (msg: string) => void,
    "onError": (msg: string) => void
  }
}

and the default would be

{
  "logger": {
    "onSuccess": console.log
    "onError": console.error
  }
}

This way the log would separate some kind "responsibility" and the log level control would be externalized

@hg-pyun
Copy link
Owner

hg-pyun commented Oct 29, 2020

@rennanboni
The current design is dependent on the axios interface.
If you want to share the error function of request and response, you may want to use the following.

instance.interceptors.request.use((request) => {
    return AxiosLogger.requestLogger(request, {
        logger: console.log
    });
}, (error) => {
    return AxiosLogger.errorLogger(error, {
        logger: conosole.error
    });
});

It would be more confusing to users if it were to be used as follows:

instance.interceptors.request.use((request) => {
    return AxiosLogger.requestLogger(request, {
        logger: {
          onSuccess: console.log
          onError: console.error
        }
    });
}, (error) => {
    return AxiosLogger.errorLogger(error, {
        logger: {
          onSuccess: console.log
          onError: console.error
        }
    });
});

@rennanboni
Copy link
Author

Hi @hg-pyun ,

Now I got it what you mean, I'll cancel this PR and create other with the follow changes

  • Add config options to errorLogger
  • Change requestLogger, responseLogger and errorLogger to use the printLog with buildConfig as optional and print log according what is available buildConfig or GlobalConfig*

Thanks for the support

@rennanboni rennanboni closed this Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants