-
Notifications
You must be signed in to change notification settings - Fork 1
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
Backend logging #535
Backend logging #535
Conversation
@@ -61,6 +62,20 @@ async function _new(req, res) { | |||
ApolloServer.prototype.createGraphQLServerOptions = _new | |||
// end of workaround | |||
|
|||
const getLogger = (uuid) => { | |||
const logToJSON = (v) => console.log(JSON.stringify(v, null, 2)) // eslint-disable-line |
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.
console.log? I was hoping we get something better, like winston. Although I haven't investigated further, I saw this discussion apollographql/apollo-server#1264 @ppershing?
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.
I can add it in subsequent PR?
@@ -69,6 +84,7 @@ const server = new ApolloServer({ | |||
}, | |||
// TODO: replace with production-ready logger | |||
formatError: (error: GraphQLError): any => { | |||
// TODO: how can we get Context here, so that we can connect error to request uuid? |
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.
I think second parameter to this function is context
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.
Thanks, will try,
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 it is not, not according to docs, and when I tried I got undefined here.
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.
Oh, Okay, then it's probably second parameter to formatResponse
I guess
No description provided.