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.

Sign up for GitHub

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

Jump to bottom

feat: user configurable logging #870

Closed
wants to merge 3 commits into from
Closed

feat: user configurable logging #870

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
807b4f8
feat(log): introduce user configurable logging
balazsorban44 Nov 20, 2020
47d922f
feat(log): use named parameters
balazsorban44 Nov 20, 2020
a5c2c18
Merge branch 'main' into feature/configurable-logging
balazsorban44 Dec 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 47 additions & 17 deletions src/lib/logger.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,61 @@
const userLogger = {
error: undefined,
warn: undefined,
debug: undefined
}

const logger = {
error: (errorCode, ...text) => {
if (!console) { return }
balazsorban44 marked this conversation as resolved.
Show resolved Hide resolved
error (code, ...text) {
if (text && text.length <= 1) { text = text[0] || '' }
console.error(
`[next-auth][error][${errorCode.toLowerCase()}]`,
text,
`\nhttps://next-auth.js.org/errors#${errorCode.toLowerCase()}`
)
code = code.toLowerCase()
const link = `https://next-auth.js.org/errors#${code}`
if (userLogger.error) {
userLogger.error(code, text, link)
} else {
console.error(
`[next-auth][error][${code}]`,
text,
`\n${link}`
)
}
},
warn: (warnCode, ...text) => {
if (!console) { return }
warn (code, ...text) {
if (text && text.length <= 1) { text = text[0] || '' }
console.warn(
`[next-auth][warn][${warnCode.toLowerCase()}]`,
text,
`\nhttps://next-auth.js.org/warnings#${warnCode.toLowerCase()}`
)
code = code.toLowerCase()
const link = `https://next-auth.js.org/warnings#${code}`
if (userLogger.warn) {
userLogger.warn(code, text, link)
} else {
console.warn(
`[next-auth][warn][${code}]`,
text,
`\n${link}}`
)
}
},
debug: (debugCode, ...text) => {
if (!console) { return }
debug (code, ...text) {
if (text && text.length <= 1) { text = text[0] || '' }
code = code.toLowerCase()
if (userLogger.debug) {
userLogger.debug(code, text)
}
if (process && process.env && process.env._NEXTAUTH_DEBUG) {
Copy link
Member Author

@balazsorban44 balazsorban44 Nov 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not checking for process.env._NEXTAUTH_DEBUG here, because if the user implemented the logger.debug method, they probably expect it to be called anyway. In this case, they can decide how debugging logs should be handled/ignored that integrates with the rest of their app's logging. (I for example have my own process.env.LOG_LEVEL env variable, which decides which logs should be sent to my third-pary and which one shouldn't. It also works in a way that when process.env.NODE_ENV is not "production", instead of sending the logs, it will fall back to console)

console.log(
`[next-auth][debug][${debugCode.toLowerCase()}]`,
`[next-auth][debug][${code}]`,
text
)
}
},
setLogger (logger = {}) {
if (typeof logger.error === 'function') {
userLogger.error = logger.error
}
if (typeof logger.warn === 'function') {
userLogger.warn = logger.warn
}
if (typeof logger.debug === 'function') {
userLogger.debug = logger.debug
}
Comment on lines +49 to +58
Copy link
Member Author

@balazsorban44 balazsorban44 Nov 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make it so that all three methods are required and only accepted together, and provide a nice TypeError to the user if they forgot one or did not define one correctly. I don't have strong opinions about this.

}
}

Expand Down
3 changes: 3 additions & 0 deletions src/server/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ if (!process.env.NEXTAUTH_URL) {
}

export default async (req, res, userSuppliedOptions) => {
if (userSuppliedOptions.logger) {
logger.setLogger(userSuppliedOptions.logger)
}
Comment on lines +25 to +27
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Providing this in userSuppliedOptions would make this totally backwards compatible and opt-in.

// To the best of my knowledge, we need to return a promise here
// to avoid early termination of calls to the serverless function
// (and then return that promise when we are done) - eslint
Expand Down