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

Plugin error logging #19

Closed
wants to merge 10 commits into from
Closed

Conversation

AndyTWF
Copy link
Contributor

@AndyTWF AndyTWF commented Jan 27, 2019

Close #18

Allows the plugin to post us JSON data regarding its state, so that we have debugging information.

);

Event::fire(new PluginErrorReceivedEvent($pluginError));
return response('', 204);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure a 204 makes sense? The response should acknowledge the creation of an event (201) or confirm all is okay (200).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to 201

*/
public function handle(PluginErrorReceivedEvent $errorReceivedEvent) : bool
{
Bugsnag::notifyException(new PluginErrorException('New plugin error reported'));
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't required; Bugsnag will catch any exception that bubbles up to the ExceptionHandler class.

use App\Events\PluginErrorReceivedEvent;
use App\Models\PluginError\PluginError;

class RecordPluginErrorInBugsnagTest extends BaseUnitTestCase
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this test class has much value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm a stickler for high test coverage, that's why it exists xD

Copy link
Contributor

Choose a reason for hiding this comment

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

If my Bugsnag comment above is addressed it should make this test (and the event) redundant

@CalumTowers
Copy link
Contributor

As discussed, we'll likely need to flesh this out a tad more.

@CalumTowers CalumTowers closed this Feb 4, 2019
@AndyTWF AndyTWF deleted the plugin-error-logging branch July 8, 2020 19: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