Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/sentry/static/sentry/bootstrap
Submodule bootstrap updated 1 files
+1 −5 README.md
50 changes: 37 additions & 13 deletions src/sentry/tasks/fetch_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import re
import urllib2
import zlib
import base64
from collections import namedtuple
from urlparse import urljoin

Expand All @@ -27,6 +28,8 @@
LINES_OF_CONTEXT = 5
CHARSET_RE = re.compile(r'charset=(\S+)')
DEFAULT_ENCODING = 'utf-8'
BASE64_SOURCEMAP_PREAMBLE = 'data:application/json;base64,'
BASE64_PREAMBLE_LENGTH = len(BASE64_SOURCEMAP_PREAMBLE)

UrlResult = namedtuple('UrlResult', ['url', 'headers', 'body'])

Expand Down Expand Up @@ -161,11 +164,15 @@ def fetch_url(url):


def fetch_sourcemap(url):
result = fetch_url(url)
if result == BAD_SOURCE:
return
if is_data_uri(url):
body = base64.b64decode(url[BASE64_PREAMBLE_LENGTH:])
else:
result = fetch_url(url)
if result == BAD_SOURCE:
return

body = result.body

body = result.body
# According to spec (https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit#heading=h.h7yy76c5il9v)
# A SourceMap may be prepended with ")]}'" to cause a Javascript error.
# If the file starts with that string, ignore the entire first line.
Expand All @@ -179,6 +186,10 @@ def fetch_sourcemap(url):
return index


def is_data_uri(url):
return url[:BASE64_PREAMBLE_LENGTH] == BASE64_SOURCEMAP_PREAMBLE


def expand_javascript_source(data, **kwargs):
"""
Attempt to fetch source code for javascript frames.
Expand Down Expand Up @@ -250,12 +261,18 @@ def expand_javascript_source(data, **kwargs):
continue

sourcemap = discover_sourcemap(result)
source_code[filename] = (result.body.splitlines(), sourcemap)

# TODO: we're currently running splitlines twice
if sourcemap:
logger.debug('Found sourcemap %r for minified script %r', sourcemap, result.url)
elif sourcemap in sourmap_idxs or not sourcemap:
if not sourcemap:
source_code[filename] = (result.body.splitlines(), None)
continue
else:
logger.debug('Found sourcemap %r for minified script %r', sourcemap[:256], result.url)

sourcemap_key = hashlib.md5(sourcemap).hexdigest()
source_code[filename] = (result.body.splitlines(), sourcemap_key)

if sourcemap in sourmap_idxs:
continue

# pull down sourcemap
Expand All @@ -264,13 +281,20 @@ def expand_javascript_source(data, **kwargs):
logger.debug('Failed parsing sourcemap index: %r', sourcemap[:15])
continue

sourmap_idxs[sourcemap] = index
if is_data_uri(sourcemap):
sourmap_idxs[sourcemap_key] = (index, result.url)
else:
sourmap_idxs[sourcemap_key] = (index, sourcemap)

# queue up additional source files for download
for source in index.sources:
next_filename = urljoin(result.url, source)
if next_filename not in done_file_list:
pending_file_list.add(next_filename)
if index.content:
source_code[next_filename] = (index.content[source], None)
done_file_list.add(next_filename)
else:
pending_file_list.add(next_filename)

has_changes = False
for frame in frames:
Expand All @@ -282,9 +306,9 @@ def expand_javascript_source(data, **kwargs):

# may have had a failure pulling down the sourcemap previously
if sourcemap in sourmap_idxs and frame.colno is not None:
state = find_source(sourmap_idxs[sourcemap], frame.lineno, frame.colno)
# TODO: is this urljoin right? (is it relative to the sourcemap or the originating file)
abs_path = urljoin(sourcemap, state.src)
index, relative_to = sourmap_idxs[sourcemap]
state = find_source(index, frame.lineno, frame.colno)
abs_path = urljoin(relative_to, state.src)
logger.debug('Mapping compressed source %r to mapping in %r', frame.abs_path, abs_path)
try:
source, _ = source_code[abs_path]
Expand Down
23 changes: 16 additions & 7 deletions src/sentry/utils/sourcemaps.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@


SourceMap = namedtuple('SourceMap', ['dst_line', 'dst_col', 'src', 'src_line', 'src_col', 'name'])
SourceMapIndex = namedtuple('SourceMapIndex', ['states', 'keys', 'sources'])
SourceMapIndex = namedtuple('SourceMapIndex', ['states', 'keys', 'sources', 'content'])

# Mapping of base64 letter -> integer value.
B64 = dict(
Expand Down Expand Up @@ -59,12 +59,10 @@ def parse_vlq(segment):
return values


def parse_sourcemap(sourcemap):
def parse_sourcemap(smap):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will leave this to the maintainers to finally decide, but python folks tend to appreciate longer, clearer variable names.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to prefer longer more explicit variable names as well, my reason for changing this was to maintain the relationship between the variable names and what the represent. In both the old and new version smap represents the source map after it has
been parsed, and sourcemap represents the unprocessed sourcemap, but I would be more than happy to use more explicit names. I was just trying to change things as little as possible

"""
Given a file-like object, yield SourceMap objects as they are read from it.
Given a sourcemap json object, yield SourceMap objects as they are read from it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"file-like object" has meaning in the python community. It means it adheres to some subset of the python File API and can be treated like a file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that makes sense, the reason I changed the doc string was cecause the function now takes the source map after it has already been parsed to json instead of taking the raw source map which is what I thought file-like object was referring to

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation and terminology are not my strong suite, if you have a suggestion for a better doc string I'd be happy to use it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes total sense. I didn't realize the api was actually changing. :)
On Oct 7, 2013 11:07 PM, "Michael Hayes" notifications@github.com wrote:

In src/sentry/utils/sourcemaps.py:

 """
  • Given a file-like object, yield SourceMap objects as they are read from it.
  • Given a sourcemap json object, yield SourceMap objects as they are read from it.

Documentation and terminology are not my strong suite, if you have a
suggestion for a better doc string I'd be happy to use it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1023/files#r6817679
.

"""

smap = json.loads(sourcemap)
sources = smap['sources']
sourceRoot = smap.get('sourceRoot')
names = smap['names']
Expand Down Expand Up @@ -108,16 +106,27 @@ def parse_sourcemap(sourcemap):


def sourcemap_to_index(sourcemap):
smap = json.loads(sourcemap)

state_list = []
key_list = []
src_list = set()
content = None

if 'sourcesContent' in smap:
content = {}
for idx, source in enumerate(smap['sources']):
if smap['sourcesContent'][idx]:
content[source] = smap['sourcesContent'][idx].splitlines()
else:
content[source] = []

for state in parse_sourcemap(sourcemap):
for state in parse_sourcemap(smap):
state_list.append(state)
key_list.append((state.dst_line, state.dst_col))
src_list.add(state.src)

return SourceMapIndex(state_list, key_list, src_list)
return SourceMapIndex(state_list, key_list, src_list, content)


def find_source(indexed_sourcemap, lineno, colno):
Expand Down
52 changes: 50 additions & 2 deletions tests/sentry/tasks/fetch_source/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
import mock

from sentry.tasks.fetch_source import (
UrlResult, expand_javascript_source, discover_sourcemap)
UrlResult, expand_javascript_source, discover_sourcemap, fetch_sourcemap)
from sentry.utils.sourcemaps import (SourceMap, SourceMapIndex)
from sentry.testutils import TestCase

base64_sourcemap = 'data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiZ2VuZXJhdGVkLmpzIiwic291cmNlcyI6WyIvdGVzdC5qcyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiO0FBQUEiLCJzb3VyY2VzQ29udGVudCI6WyJjb25zb2xlLmxvZyhcImhlbGxvLCBXb3JsZCFcIikiXX0='


class DiscoverSourcemapTest(TestCase):
# discover_sourcemap(result)
Expand Down Expand Up @@ -42,7 +45,8 @@ class ExpandJavascriptSourceTest(TestCase):
@mock.patch('sentry.models.Event.update')
@mock.patch('sentry.tasks.fetch_source.fetch_url')
@mock.patch('sentry.tasks.fetch_source.fetch_sourcemap')
def test_simple(self, fetch_sourcemap, fetch_url, update):
@mock.patch('sentry.tasks.fetch_source.discover_sourcemap')
def test_simple(self, discover_sourcemap, fetch_sourcemap, fetch_url, update):
data = {
'sentry.interfaces.Exception': {
'values': [{
Expand All @@ -65,6 +69,7 @@ def test_simple(self, fetch_sourcemap, fetch_url, update):
}],
}
}
discover_sourcemap.return_value = None
fetch_sourcemap.return_value = None
fetch_url.return_value.body = '\n'.join('hello world')

Expand All @@ -82,3 +87,46 @@ def test_simple(self, fetch_sourcemap, fetch_url, update):
assert frame['pre_context'] == []
assert frame['context_line'] == 'h'
assert frame['post_context'] == ['e', 'l', 'l', 'o', ' ']

@mock.patch('sentry.models.Event.update')
@mock.patch('sentry.tasks.fetch_source.fetch_url')
@mock.patch('sentry.tasks.fetch_source.discover_sourcemap')
def test_inlined_sources(self, discover_sourcemap, fetch_url, update):
data = {
'sentry.interfaces.Exception': {
'values': [{
'stacktrace': {
'frames': [
{
'abs_path': 'http://example.com/test.js',
'filename': 'test.js',
'lineno': 1,
'colno': 0,
},
],
},
}],
}
}
discover_sourcemap.return_value = base64_sourcemap
fetch_url.return_value.body = '\n'.join('<generated source>')

expand_javascript_source(data)
fetch_url.assert_called_once_with('http://example.com/test.js')

frame_list = data['sentry.interfaces.Exception']['values'][0]['stacktrace']['frames']
frame = frame_list[0]
assert frame['pre_context'] == []
assert frame['context_line'] == 'console.log("hello, World!")'
assert frame['post_context'] == []


class FetchBase64SourcemapTest(TestCase):
def test_simple(self):
index = fetch_sourcemap(base64_sourcemap)
states = [SourceMap(1, 0, '/test.js', 0, 0, None)]
sources = set(['/test.js'])
keys = [(1, 0)]
content = {'/test.js': ['console.log("hello, World!")']}

assert index == SourceMapIndex(states, keys, sources, content)
5 changes: 4 additions & 1 deletion tests/sentry/utils/sourcemaps/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
find_source)
from sentry.testutils import TestCase

from sentry.utils import json


sourcemap = """{"version":3,"file":"file.min.js","sources":["file1.js","file2.js"],"names":["add","a","b","multiply","divide","c","e","Raven","captureException"],"mappings":"AAAA,QAASA,KAAIC,EAAGC,GACf,YACA,OAAOD,GAAIC,ECFZ,QAASC,UAASF,EAAGC,GACpB,YACA,OAAOD,GAAIC,EAEZ,QAASE,QAAOH,EAAGC,GAClB,YACA,KACC,MAAOC,UAASH,IAAIC,EAAGC,GAAID,EAAGC,GAAKG,EAClC,MAAOC,GACRC,MAAMC,iBAAiBF"}"""

Expand All @@ -30,7 +32,8 @@ def test_simple(self):

class ParseSourcemapTest(TestCase):
def test_basic(self):
states = list(parse_sourcemap(sourcemap))
smap = json.loads(sourcemap)
states = list(parse_sourcemap(smap))

assert states == [
SourceMap(dst_line=0, dst_col=0, src='file1.js', src_line=0, src_col=0, name=None),
Expand Down