-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
Server should not log to console.debug #39
Comments
I'm not familiar with all the frameworks out there, but can't you simply filter the logging levels you are interested in? It seems kind of weird to me that any lib should support customizing logging verbosity, instead of filtering the logging levels for the whole application. I'd be interested in hearing about how others are doing it. |
Its very uncommon for a npm library to use console log without any conditions/switches. Using "debug" (https://www.npmjs.com/package/debug) is very common where you can control logging via an environment variable: import Debug from 'debug';
const debugLog = Debug('mypackage');
debugLog('Debug information...'); (it also works in the browser but its more common in NodeJS) Also IMHO for anything related to security (like webauthn) just logging everthing to console (which logs to stdout in node) is not a good idea, even though in this case its probably not an issue Thanks |
Regarding the logging lib, it seems like the ecosystem is very fragmented currently, so I simply won't pick any. However, as you said, it would be bad having sensitive information accidentally appearing in logs. Therefore I'll disable all debug logs by default and activate them only if the existing |
Great, that should work for us - And yes the ecosystem is very fragmented which is more or less the default state for npm/JS |
Disabled by default in latest release and added |
verifySignature()
logs various things toconsole.debug()
but there is no setting to turn this off other than overridingconsole.debug
Some kind of setting would be helpful here because debug info isn't required most of the time.
Thanks for the awesome library 😃
The text was updated successfully, but these errors were encountered: