Skip to content

Conversation

@plibither8
Copy link

This PR is partially completes https://github.com/sourcegraph/sourcegraph/issues/26582. Next step would be to create a PR in the sourcegraph repo to console.error errors captured by Sentry.


This rule bans logging errors directly to the console using console.error() calls, unless supported with a comment explaining why it's required. Otherwise, it's recommended to pass the error through Sentry.captureException().

Bad code

try {
  // do something
} catch (error) {
  console.error(error)
}

Okay code

try {
  // do something
} catch (error) {
  // We need to log this to the browser console because XYZ
  console.error(error)
}

(Bonus) Awesome code

try {
  // do something
} catch (error) {
  Sentry.captureException(error)
}

Code 2022-07-08 at 17 00 18 000174@2x

Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Looks great! Let's decide on the error message, and it's good to go 🙌


export const messages = {
noUnexplainedConsoleError:
'Directly logging through `console.error()` is discouraged. If required, please add a comment explaining why so, otherwise consider passing the error through `Sentry.captureException()` instead.',
Copy link
Member

Choose a reason for hiding this comment

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

Depending on the outcome of the discussion in the follow-up PR, we would need to update the message here. Using Sentry.captureException directly is not safe because it's not always available. We would want to point consumers to something more generic that encapsulates Sentry and other output channels.

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.

3 participants