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

Capture messages without timestamps in the title #78

Closed

Conversation

HJGreen
Copy link
Contributor

@HJGreen HJGreen commented Sep 28, 2021

Allows the Sentry UI to properly group related issues. The formatted property includes a timestamp, which causes Sentry to log them as separate issues.

See the attached screenshot for an example. There are two missing CSS files which Silverstripe is trying to link into the page. The first four entries show the current behaviour where each time the page is loaded, a new Issue is logged in Sentry due to the timestamp.

The bottom two rows are with the PR applied - similar events are grouped with the original.

Screen Shot 2021-09-28 at 5 07 09 PM

Allows the Sentry UI to properly group related issues.

The 'formatted' property includes a timestamp, which causes Sentry to log them as separate issues.
@@ -138,7 +138,7 @@ protected function write(array $record): void
}

$this->client->captureMessage(
$record['formatted'],
$record['context']['message'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I invoke manual logging - there is no "message" key in the "context" array:

        $logger = Injector::inst()->createWithArgs(Logger::class, ['error-log'])
            ->pushHandler(SentryHandler::create('DEBUG')); // Send errors >= DEBUG
        $logger->info('This is a test');

[Notice] Undefined index: message

It would be good to see exactly how you triggered the error (i.e. is bundle.css being called in a template context or via the Requirements API?)

The following might work better:

$record['context']['message'] ?? $record['formatted']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case the CSS files were being included via the Requirements API:

    protected function init()
    {
        parent::init();
        Requirements::css('themes/base/build/bundle-index.css');
        Requirements::css('themes/base/build/bundle-print.css', 'print');
    }

There is a $record['message'] property too - which contains the following:
E_USER_NOTICE: File themes/base/build/bundle-index.css does not exist

So perhaps?
$record['context']['message'] ?? $record['message']

phptek added a commit that referenced this pull request Oct 1, 2021
Also prevents Exceptions from logging > once.
@phptek
Copy link
Owner

phptek commented Oct 1, 2021

@HJGreen thanks for this - using "message" actually sorts the problem. In the process of looking at this, I realised that exceptions were also logging twice for each message. I have fixed this too - currently in master.

@phptek phptek closed this Oct 1, 2021
@phptek
Copy link
Owner

phptek commented Oct 4, 2021

@HJGreen FYI, you can use tag 4.0.5 which incorporates this fix.

@HJGreen HJGreen deleted the feature/fix-duplicate-message-reporting branch October 5, 2021 01:24
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.

None yet

3 participants