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

Sentry logs not capturing errors from App Store or GEO Diary #48

Closed
HollyJoyPhillips opened this issue Jan 22, 2020 · 5 comments · Fixed by #169
Closed

Sentry logs not capturing errors from App Store or GEO Diary #48

HollyJoyPhillips opened this issue Jan 22, 2020 · 5 comments · Fixed by #169
Assignees
Labels
bug Something isn't working front-end Relates to front end issues geo-diary Relates to GEO Diary app marketplace Relates to the Marketplace

Comments

@HollyJoyPhillips
Copy link
Contributor

Summary: Errors created in the App Store and GEO Diary are not being captured by Sentry.

Expected Behaviour: Errors created should be logged and notifications sent through the channel

Errors & Images:

The following error was created in App Store by trying to submit an app whilst reaching the limit of 'Unlisted' apps:
image

The following error was created in GEO Diary by trying to cancel an appointment (_etag issue):
image

@HollyJoyPhillips HollyJoyPhillips added bug Something isn't working marketplace Relates to the Marketplace geo-diary Relates to GEO Diary app labels Jan 22, 2020
duong-se added a commit that referenced this issue Jan 22, 2020
* [CLD-345] Integrate API for agent check

* Fixes test
willmcvay added a commit that referenced this issue Jan 24, 2020
willmcvay pushed a commit that referenced this issue Jan 24, 2020
@phmngocnghia phmngocnghia self-assigned this Jan 29, 2020
@duong-se duong-se self-assigned this Jan 31, 2020
@duong-se
Copy link
Contributor

duong-se commented Jan 31, 2020

@HollyJoyPhillips now the error capture when ever front-end have crash bug by code not by the backend return result. So do you want to capture error from backend also?

@vuhuucuong
Copy link
Contributor

I believe Sentry only captures Uncaught errors, which are errors that are thrown without catch, so that we can track down the error to handle it correctly. Since all the errors in Holly's example have been caught and show an appropriate error message, that will not be logged to Sentry. So I think that's expected behavior.
What do you think @willmcvay ?

@willmcvay
Copy link
Contributor

Can we log caught errors as info? Would be good to get some feedback on how our users are interacting with the product in production, where they are going wrong etc. Just a small logging module to replace the console.error in our catch blocks eg

const logger = (err: Error) => {
if (process.env.NODE_ENV === 'production`) {
     // send an event to Sentry
} else {
   console.error(err.message)
}
}

Usage:

logger(err.message)

@willmcvay
Copy link
Contributor

Obviously the above is pseudo code but you get the idea :-)

@vuhuucuong vuhuucuong assigned vuhuucuong and unassigned duong-se Feb 3, 2020
@vuhuucuong
Copy link
Contributor

vuhuucuong commented Feb 3, 2020

Okay, I got the idea. So that means we need to apply it to all apps, including AML and LTL, right? @willmcvay

vuhuucuong added a commit that referenced this issue Feb 12, 2020
vuhuucuong added a commit that referenced this issue Feb 12, 2020
vuhuucuong added a commit that referenced this issue Feb 12, 2020
duong-se pushed a commit that referenced this issue Feb 12, 2020
)

* fix: #48 sentry log not capture error, add logger util in all apps

* fix: #48 fix test file

* fix: #48 move logger utils to root

* fix: #48 resolve conflict
nphivu414 pushed a commit that referenced this issue Apr 29, 2020
)

* fix: #48 sentry log not capture error, add logger util in all apps

* fix: #48 fix test file

* fix: #48 move logger utils to root

* fix: #48 resolve conflict
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working front-end Relates to front end issues geo-diary Relates to GEO Diary app marketplace Relates to the Marketplace
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants