Added support for minified react exceptions #3632
Conversation
| timeout=settings.SENTRY_SOURCE_FETCH_TIMEOUT, | ||
| ) | ||
| data = response.json() | ||
| cache.set(key, json.dumps(data), 120) |
mitsuhiko
force-pushed
the
feature/react-minified-errors
branch
from
cd94874
to
efdb416
Compare
| http_session = http.build_session() | ||
| response = http_session.get(self.mapping_url, | ||
| allow_redirects=True, | ||
| verify=False, |
There was a problem hiding this comment.
Why are we skipping TLS validation here?
There was a problem hiding this comment.
@mattrobenolt why are we for sourcemaps?
There was a problem hiding this comment.
I just did what you did for sourcemaps :)
There was a problem hiding this comment.
Yeah, that's fine. Just making sure since we're choosing to explicitly opt-out, but it's probably reasonable here and the risk is really really low.
There was a problem hiding this comment.
Oh, but in this case, this is explicitly pulling from GitHub. They should have valid TLS certificate so I think it'd be more valuable to verify since we use this as a source for mappings.
|
Reviewed 3 of 3 files at r1. tests/sentry/lang/javascript/test_processor.py, line 294 [r1] (raw file):
only thing blocking for me is changing this to use a responses fixture Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed. tests/sentry/lang/javascript/test_processor.py, line 294 [r1] (raw file):
|
|
I mocked out the test now. Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed. Comments from Reviewable |
|
Reviewed 2 of 2 files at r2. Comments from Reviewable |
mitsuhiko
force-pushed
the
feature/react-minified-errors
branch
from
e40b351
to
a65f140
Compare
|
Ready to merge? Review status: 2 of 3 files reviewed at latest revision, 1 unresolved discussion. src/sentry/lang/javascript/errormapping.py, line 35 [r1] (raw file):
|
|
I changed this now to have a two-stage cache. We try to renew the data every 5 minutes but if we fail we fall back for up to an hour on older data. Also verify is now now :P Review status: 1 of 3 files reviewed at latest revision, 1 unresolved discussion. Comments from Reviewable |
|
Reviewed 2 of 2 files at r4. src/sentry/lang/javascript/errormapping.py, line 17 [r4] (raw file):
I would probably just make this a super super long time (like one week) Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 2 unresolved discussions. src/sentry/lang/javascript/errormapping.py, line 17 [r4] (raw file):
|
|
Review status: 2 of 3 files reviewed at latest revision, 2 unresolved discussions. src/sentry/lang/javascript/errormapping.py, line 35 [r1] (raw file):
|
|
Review status: 2 of 3 files reviewed at latest revision, 1 unresolved discussion. src/sentry/lang/javascript/errormapping.py, line 35 [r1] (raw file):
|
|
Reviewed 1 of 3 files at r1, 1 of 2 files at r4, 1 of 1 files at r5. Comments from Reviewable |
This looks up the minified react error codes and expands the message. It
fetches the error map from github and keeps it cached for two minutes. We
could expand that if we want.
This change is