Add meta data aware interface and trait

migo315 commented 4 years ago

Goal

Provide a very simple way to extend an exception with additional meta data everywhere without further dependencies.

Design

This feature is very pragmatic and straightforward. Similar to PSR Logger, we provide a MetaDataAwareInterface and a MetaDataAwareTrait. The developer can (but must not) extend his Exceptions with the Interface / Trait to add some additional informations:

class UnrecoverableProcessException extends Exception implements MetaDataAwareInterface
{
    use MetaDataAwareTrait;
}

Later just pass your informations:

$exception = new UnrecoverableProcessException('I am an Exception');
$exception->addMetaData(['myTab' => [
    'foo' => 'bar',
]]);

throw $exception;

This implementation does not need any services or heavy structures.

Changeset

Added

MetaDataAwareInterface
MetaDataAwareTrait

Changed

BugsnagListener add MetaData if Exception implements interface

Tests

I have added automatic tests (with and without meta data)

Review

For the submitter, initial self-review:

[x] Commented on code changes inline explain the reasoning behind the approach
[x] Reviewed the test cases added for completeness and possible points for discussion
[x] A changelog entry was added for the goal of this pull request
[x] Check the scope of the changeset - is everything in the diff required for the pull request?
This pull request is ready for:
- [ ] Initial review of the intended approach, not yet feature complete
- [ ] Structural review of the classes, functions, and properties modified
- [x] Final review

For the pull request reviewer(s), this changeset has been reviewed for:

[ ] Consistency across platforms for structures or concepts added or modified
[ ] Consistency between the changeset and the goal stated above
[ ] Internal consistency with the rest of the library - is there any overlap between existing interfaces and any which have been added?
[ ] Usage friction - is the proposed change in usage cumbersome or complicated?
[ ] Performance and complexity - are there any cases of unexpected O(n^3) when iterating, recursing, flat mapping, etc?
[ ] Concurrency concerns - if components are accessed asynchronously, what issues will arise
[ ] Thoroughness of added tests and any missing edge cases
[ ] Idiomatic use of the language

mattdyoung commented 4 years ago

Hi @migo315

Thanks for the PR. We plan to look into this and consider whether we want to use this approach across our PHP integrations.

migo315 commented 4 years ago

I can split this this if you like. Interface and Trait to bugsnag-php. Then all additional php libraries can use the Interface and Trait (Symfony, Laravel etc).

mattdyoung commented 4 years ago

@migo315 That sounds really useful if you have the time. To be clear, with our current priorities it will be at least a few weeks until we're able to have a detailed look at your PR and there would be work on our side to document this and ensure we're happy with the interface and test coverage. So in the short term you may want to fork the repository to make use of your own changes, but we do like the approach so are likely to take this on once we have the bandwidth.

Also, a word of caution. We currently support PHP 5.2+ and I believe Traits only came in 5.4, so we'd need to ensure we consider the best way to handle that.

migo315 commented 4 years ago

I've created an PR for your PHP SDK https://github.com/bugsnag/bugsnag-php/pull/574

johnkiely1 commented 4 years ago

This has now been fixed via php release V3.21.0 https://github.com/bugsnag/bugsnag-php/releases/tag/v3.21.0. running composer update bugsnag/bugsnag will pull in latest version

We have added support for the getting the original error on which you can attach additional metadata.

bugsnag / bugsnag-symfony