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.

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

Log arguments in extra so that they do not need to be stringified #398

Merged
merged 3 commits into from
Oct 31, 2015

Conversation

tarjei
Copy link
Contributor

@tarjei tarjei commented Oct 30, 2015

Log arguments in extra so that they do not need to be stringified. This provides more context for use when debugging errors.

Log arguments in extra so that they do not need to be stringified. This provides more context for use when debugging errors.
@@ -23,7 +23,7 @@ var logForGivenLevel = function(level) {
if (level === 'warn') level = 'warning';
return function () {
var args = [].slice.call(arguments);
Raven.captureMessage('' + args, {level: level, logger: 'console'});
Raven.captureMessage('' + args[0], {level: level, logger: 'console', extra: { args: args }});
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we want to do { args: args.slice(1) } instead? Since this is already sending the first argument as the message.

Also, I think I'd prefer to send the key as arguments rather than args since arguments is what it's really called. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re args.slice(1): I would prefer to keep all all the arguments in extras because you can have a complex object in the first argument of console.xxx() as well. You do not want to have to recommit your code just because you forgot that the first argument should be a string...

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that's probably reasonable. 👍

So the intent becomes more clear.
@mattrobenolt
Copy link
Contributor

Thanks! 🍰 🎉

mattrobenolt added a commit that referenced this pull request Oct 31, 2015
Log arguments in extra so that they do not need to be stringified
@mattrobenolt mattrobenolt merged commit b45aec4 into getsentry:master Oct 31, 2015
@tarjei
Copy link
Contributor Author

tarjei commented Oct 31, 2015

Thank you for accepting the branch - and THANK YOU for RavenJS and Sentry!

@tarjei tarjei deleted the patch-1 branch October 31, 2015 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants