-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: add support for system logger #85
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we consider using a library for the logger? Like winston or pino? Might be an overkill, but both have support for multiple transport, so different log levels can be routed to different transports.
Anyway, might be nice in the future to pass this down to zisi too, so we can log more debug messages there too. :)
Sorry @danez, I rejigged things quite a bit, because I was holding this wrong. I think more than debug levels (which indeed we'd probably be better off getting from a logging library), we need the ability to differentiate user-facing errors from system errors. Can you have another look at this PR, please? I'll be happy to answer any questions that you may still have after reviewing. |
|
||
return { | ||
system, | ||
user: console.log, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not use that anywhere yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't. I thought it'd be nice to have it here for when we do need it, but I can also remove it if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No all fine just wondering.
* feat: add support for custom logger * refactor: introduce user and system loggers
Which problem is this pull request solving?
With netlify/build#3220 in the pipeline, we'll be able to capture debug data into our system logs without polluting customer builds logs. This PR takes advantage of that and introduces an optional
logFunction
to bothbundle
andserve
, which is then used instead ofconsole.log
to print logs.When this is in place, Netlify Build will be able to pass along the
systemLog()
utility, which was introduced in netlify/build#4406.