Closed sentry-io[bot] closed 4 months ago
@johnsmith-gooddollar we are prioritizing this for sprint 6 - can you confirm you are accountable and will own this ticket?
@L03TJ3
Should we log or not? @johnsmith-gooddollar @sirpy analyzed a bunch of the reported issues
syncFromExplorer:
Something with codepusH?: codepush-event
and the question.. should we completely hide network errors from logging: https://github.com/GoodDollar/GoodDAPP/pull/4192 or are there places where some of the above information is relevant?
@L03TJ3 warnings are irrelevant we are only talking about errors which are logged to sentry. Usually i'd say network errors are related to user connection problems but it might be also be an issue with a specific url/domain, temporarily down, or maybe a typo.
@johnsmith-gooddollar @L03TJ3 if we can log the url that failed it might make more sense, and also perhaps make sure we log the url only once per session/refresh
@sirpy if any errors are caused by due to faulty rpc errors and are not picked up by any other the faulty rpc's will be shown in the stack trace on sentry still. no need to log them all imo
The ones regularly complaining about threshold are: nodies onfinality
at least the ones I noticed
@L03TJ3 if for example we put a wrong rpc address with a typo, the logger will ignore any errors with this rpc. what make you say that this errors will be logged elsewhere?
@sirpy What I basically meant was that if there is an error from the app caused by a faulty rpc, then the stacktrace would show all the rpc's that were attempted, and we could catch/resolve if there a rpc failing cause of a human error or something.
besides that, what I now see is that any error/warning has been silenced and wrapped with !isConnectionError, so whatever meant is not usefull anyway
reference which warnings I would expect to see in a stack strace: https://github.com/GoodDollar/GoodDAPP/blob/4e98a611109011a1281dd924c8f26b2148ca673b/src/lib/wallet/MultipleHttpProvider.js#L69
@L03TJ3 warnings are irrelevant we are only talking about errors which are logged to sentry. Usually i'd say network errors are related to user connection problems but it might be also be an issue with a specific url/domain, temporarily down, or maybe a typo.
@johnsmith-gooddollar @L03TJ3 if we can log the url that failed it might make more sense, and also perhaps make sure we log the url only once per session/refresh
@johnsmith-gooddollar can we do that? log url that failed once?
@johnsmith-gooddollar
@sirpy @L03TJ3 After some additional research I've found it's not easy to do And even impossible in some cases because we cannot always get failing url from the exception :
a) we could get failed URL from axios error b) we could understand which http provider failed
In the other cases (e.g. failed fetch() to XMLHTTPRequest() from some liberary like realmdb) we cannot obtain url has failed with the network error.
So what I've done
@johnsmith-gooddollar i've added comments on the commits. please use PR next time
@sirpy
this should be per url not host
Fixed
log.error? but now it will not single time eerrors about urls
restore pcallback it is overwriting another function called callback
Fixed
@johnsmith-gooddollar i've added comments on the commits. please use PR next time
@vldkhh verify on prod
@L03TJ3 need to recheck - https://gooddollar.sentry.io/issues/2845295278/?project=1829798&referrer=github_integration
@vldkhh I think this can be rechecked as well
verified on prod
I've merged 4 network error issues to this one (you can see them by clicking the merged issues tab in sentry)
Can we safely ignore and not report Network error and network request error?
Sentry Issue: GOODDAPP-5VXW