Support notification level callbacks

[nkavian]

Description

My original problem is that I want the post body to be logged with each notification to Bugsnag. Since the Java library doesn't do this, I have to hack together a solution on my side.

It's an expensive task to take the request body my controller received and then serialize it back into a string and attach it to Bugsnag using addThreadMetaData. If the exception rarely occurs, I would be wasting time on this expensive operation on all successful requests.

Describe the solution you'd like

I'd like addThreadMetaData to take a callback (or a runnable lambda) instead of an Object, so that Bugsnag can evaluate it only if it needs to perform the notification to Bugsnag.

Describe alternatives you've considered

Although the method bugsnag.addCallback gives access to the granular report, the callback is created when Bugsnag is initialized. The callback has no way to be aware of the controller state in Spring.

Additional context

My Spring controller decodes the post body into a specific object, for example

void postRequest(@RequestBody Payload payload) {}

I tried addThreadMetaData(..., payload) but Bugsnag strips the contents. You're using an object mapper that filters for example things that aren't marked with your Expose annotation.

An alternate solution to this thread: you could continue to use your existing object mapper for your own objects, but use a second new object mapper for the objects added via addThreadMetaData so that you stop stripping out the data.

[xljones]

Hey @nkavian, based on the original problem and title of the issue, it sounds like you should be able to use a local beforeNotify() callback to add metadata in a call to bugsnag.notify() where you could serialize and add the request body, as at this point you should be in scope of the controller in Spring and have access to the request:

bugsnag.notify(exception, new Callback() {
    @Override
    public void beforeNotify(Report report) {
		// serialize the request body here..
        report.addToTab("request", "body", serialized_request);
    }
});

Or you could use a Java lambda syntax: https://docs.bugsnag.com/platforms/java/spring/reporting-handled-exceptions/#setting-metadata. There's a plain Java example here, and an example in Spring too (although it uses the same underlying library).

On a side-note; just make sure that including the body of the request doesn't push the payload to Bugsnag over the 1MB limit, as the report would otherwise be rejected.

[nkavian]

Hi @xander-jones, those examples use handled exceptions. In my case, I am trying to support an unhandled exception.

[johnkiely1]

Hi @nkavian,

Perhaps I'm misunderstanding your use case, but presumably you are only interested in the post body when the request fails? If so you could try/catch the request at that point it is made from the controller and then notify similar to what Xander suggested above.

If this is not the case it may be worth explaining a little more about your code structure and existing error handling to help us understand why you would expect these to be unhandled exceptions?

[nkavian]

@johnkiely1 I'm not sure where the confusion is coming from, so I'll break this down into more details. Pardon me being more verbose.

Let's take this very basic example:

    @PostMapping("{userId}")
    public void updateUser(@PathVariable UUID userId, @RequestBody User user) {
        String fail = null;
        fail.toString();
    }

If fail.toString() is executed, Bugsnag will catch the NullPointerException as an unhandled exception and it will report it. My first question is "Why aren't you capturing the post request body like you do with other SDKs?"

Next, wrapping the body of that method with try/catch defeats the purpose of using Bugsnag to capture unhandled exceptions. No one can be expected to go through all their code to wrap their methods like this:

    @PostMapping("{userId}")
    public void updateUser(@PathVariable UUID userId, @RequestBody User user) {
        try {
            String fail = null;
            fail.toString();
        } catch (Exception e) {
            // do something expensive here to read the request body as bytes
            // do something expensive to convert the bytes into a string: "theRequestBodyContent"
            Bugsnag.addThreadMetaData("Context", "Request Body", theRequestBodyContent);
            bugsnag.notify(exception, Severity.ERROR);
        }
    }

Next, imagine if I could rewrite the above based on a new feature where addThreadMetaData could accept a lambda. imagine the lambda is executed only when a report is triggered. This code also decouples the meta data from the rest of the code.

    @PostMapping("{userId}")
    public void updateUser(@PathVariable UUID userId, @RequestBody User user) {
        Bugsnag.addThreadMetaData("Context", "Request Body", () -> {
            // do something expensive here to read the request body as bytes
            // do something expensive to convert the bytes into a string: "theRequestBodyContent"
            return theRequestBodyContent;
        });

        String fail = null;
        fail.toString();
    }

Next, wrapping the method bodies with try/catch will not capture all exceptions, for example if the JSON post body is malformed or validation on the User object fails.

So 3 problems:
1 - I need the HTTP body captured as part of reports sent to Bugsnag
2 - Generating / capturing the HTTP body is expensive
3 - And, I need number 2 to only happen if number 1 occurs.

So here would be my solution:

First, implement a ContentCachingRequestWrapper (i.e. in my code) as described here so that the HTTP body can be read more than just once.

Second, ideally, use a lambda to attach the HTTP body only when there is an error (i.e. a new Bugsnag feature).

Here's the full example:

    // The same original method I showed above. No changes.
    @PostMapping("{userId}")
    public void updateUser(@PathVariable UUID userId, @RequestBody User user) {
        String fail = null;
        fail.toString();
    }


    @Component
    public class CachingRequestBodyFilter extends GenericFilterBean {

        @Override
        public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain chain) {
            HttpServletRequest currentRequest = (HttpServletRequest) servletRequest;
            ContentCachingRequestWrapper wrappedRequest = new ContentCachingRequestWrapper(currentRequest);

            // The lambda should only be evaluated if/when a report to Bugsnag is triggerred.
            Bugsnag.addThreadMetaData("Context", "Request Body", () -> {
                // do something expensive here to read the request body as bytes
                // do something expensive to convert the bytes into a string
                return new String(requestWrapper.getContentAsByteArray());
            });

            chain.doFilter(wrappedRequest, servletResponse);
        }

    }

In response to the question around: Could I call bugsnag.notify with a callback.

bugsnag.notify(exception, new Callback() {
    @Override
    public void beforeNotify(Report report) {
		// serialize the request body here..
        report.addToTab("request", "body", serialized_request);
    }
});

I have a few points:

• This doesn't solve for unhandled exceptions. This assumes only handled exceptions.
• There is coupling happening there. If I have an exception in hand, why would I go out of my way to create a lambda callback, why wouldn't I just call addThreadMetaData in the first place, i.e.:

Bugsnag.addThreadMetaData("Context", "First Name", firstName);
Bugsnag.addThreadMetaData("Context", "Last Name", lastName);
bugsnag.notify(exception, Severity.ERROR);

If that coupling wasn't there, and you removed the exception, and just took a callback to be used with all notifications, that would be the same feature I'm describing above, i.e.:

bugsnag.notify(new Callback() {
    @Override
    public void beforeNotify(Report report) {
        // ** this method called for all exceptions **
        // serialize the request body here..
        report.addToTab("request", "body", serialized_request);
    }
});