-
Notifications
You must be signed in to change notification settings - Fork 55
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 disableServerLogging option #39
base: main
Are you sure you want to change the base?
Add disableServerLogging option #39
Conversation
test/test.js
Outdated
@@ -128,4 +128,47 @@ describe('beaver-logger tests', () => { | |||
} | |||
}); | |||
}); | |||
|
|||
it('should not send event to server if enableServerLogging is false', () => { |
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.
if disableServerLogging is true
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.
Fixed
test/test.js
Outdated
logEndpoint.expectCalls(); | ||
return $logger.flush().then(() => { | ||
try { | ||
logEndpoint.done(); // will throw is not API calls received by logEndpoint |
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.
// will throw if API calls received by logEndpoint
. How will it throw? I'm guessing you are referring to the if
check below if the handler was called. If so, this comment is confusing since we //pass in the catch.
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.
https://github.com/krakenjs/sync-browser-mocks/blob/master/README.md#xmlhttprequest
it('should correctly call /api/user', function() {
myListener.expectCalls();
// Run some code which would call the api
myListener.done(); // This will throw if the api was not called
});
I can set a variable in the catch block to indicate that an exception is indeed thrown.
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.
👍
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.
Done
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.
Looks great! Thanks, @pushpinder107! 🎉
Can this be done an a per level basis? I would like to avoid sending debug logs to the server side unless they are enabled. |
Addresses #19 by adding
disableServerLogging
option.Usage
const logger = Logger({ url: '/test/api/log', disableServerLogging: true });