BugsnagJS integration is not logging all errors

[luisramos0]

Description

There are Javascript errors in the checkout page that are not appearing on Bugsnag.

From discussion in https://github.com/openfoodfoundation/openfoodnetwork/issues/5785 I setup my local environment with Bugsnag AUS keys both base api key and JS.

export BUGSNAG_API_KEY=... export BUGSNAG_JS_KEY=...

I manage to get alerts on both server account and JS account. Server: https://app.bugsnag.com/yaycode/openfoodnetwork/errors/5f2860ac9e4eb600186253ab?filters[event.since][0]=30d&filters[error.status][0]=open JS: https://app.bugsnag.com/yaycode/open-food-network-js-australia/errors/5e830917206643001787799f?filters[event.since][0][type]=eq&filters[event.since][0][value]=30d&filters[error.status][0][type]=eq&filters[error.status][0][value]=open&filters[app.release_stage][0][value]=development&filters[app.release_stage][0][type]=eq

Re the server side alert, I get it by simply raising an exception on CheckoutController#update.

On the JS side, although I did manage to get an alert in bugsnag, things are not really working. In Checkout.js#submit I add: Checkout fails and I see the error in the browser console. The error is not found on bugsnag.

This makes me conclude Bugsnag JS is not really picking up all the errors.

Expected Behavior

All JS errors are reported in bugsnagJS.

Actual Behaviour

No alert in bugsnag.

Steps to Reproduce

See description.

Workaround

No data about the errors.

Severity

bug-s2, in #5785 we kind of concluded this is an S2 as we do need info about all JS errors related to payments so that we are aware if we have a lot of customers experiencing problems. I am not sure about this.

Your Environment

local v3.2.1

Possible Fix

Some bugsnagJS config?

[Matt-Yorkley]

I've read through all the docs on configuration and can't find any reason it wouldn't report all errors, it's supposed to by default.

I've played around with this a lot but haven't got any useful results, other than noticing that Bugsnag JS has a new major version (v7) which apparently brings a lot of improvements. I've made a PR to update it, but I'm not sure it resolves the issue.

[Matt-Yorkley]

I'm also looking at the syntax in https://github.com/openfoodfoundation/openfoodnetwork/pull/5202 and I'm not sure the use of .catch is correct, based on the docs for Angular 1.5.8: https://code.angularjs.org/1.5.8/docs/api/ng/service/$http ?

[Matt-Yorkley]

I haven't managed to figure out why the unhandled errors are not all being passed to Bugsnag, but I've opted to explicitly send checkout errors with Bugsnag.notify() and have verified they are being sent/received.

[luisramos0]

thanks Matt!

re the change in 5202, $http returns a promise you can see catch here, it's a shortcut: https://code.angularjs.org/1.5.8/docs/api/ng/service/$q

I researched it at the time, I found people saying that catch works better in more browsers, I added that to the commit message here: https://github.com/openfoodfoundation/openfoodnetwork/pull/5202/commits/7414047b924eb0cf241b7ceb108f88ddd8468a0c

[luisramos0]

I see you reverted it in the new PR, I am ok with that :+1:

[luisramos0]

So, it looks like #5877 does not close this issue (log all errors). 5877 makes the situation much better and it does add an explicit call to bugsnag on JS checkout errors.

To get all errors and close this issue we may need to fix some config issue...

Meanwhile we can add some more Bugsnag.notify entries to mitigate the problem. I think we ca add an explicit call to bugsnag on stripe_elements:

Which is related to making the stripe elements call a bit more resilient suggested here to help improve #5785

I think this explicit call to bugsnag on the stripe elements code would make this issue #5858 an S3. For now I think still S2, I am moving this to top of Dev Ready. Does this make sense?

[luisramos0]

So,I think #5951 makes this an S3 when merged.

[luisramos0]

So, I think this is not really "Dev Ready", this is more like in "Core Review" now with #5951 and when 5951 is merged, this can be moved to "All the things" as an S3.