Skip to content

Commit 5b9f294

Browse files
author
Riccardo Busetti
authored
fix(sourcemaps): Fix contraint violation during creation of DebugIdArtifactBundle entries (#47310)
1 parent 0e5a5e5 commit 5b9f294

File tree

9 files changed

+211
-5
lines changed

9 files changed

+211
-5
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
/* eslint-disable */
2+
function helloWorld() {
3+
alert('HelloWorld');
4+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
/* eslint-disable */
2+
function helloWorld(){alert("HelloWorld")}

fixtures/artifact_bundle_duplicated_debug_ids/files/_/_/bundle1.min.js.map

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/* global exports */
2+
Object.defineProperty(exports, '__esModule', {value: true});
3+
const tslib_1 = require('tslib');
4+
const hub_1 = require('@sentry/core');
5+
/**
6+
* This calls a function on the current hub.
7+
* @param method function to call on hub.
8+
* @param args to pass to function.
9+
*/
10+
function callOnHub(method) {
11+
const args = [];
12+
for (let _i = 1; _i < arguments.length; _i++) {
13+
args[_i - 1] = arguments[_i];
14+
}
15+
const hub = hub_1.getCurrentHub();
16+
if (hub && hub[method]) {
17+
// tslint:disable-next-line:no-unsafe-any
18+
return hub[method].apply(hub, tslib_1.__spread(args));
19+
}
20+
throw new Error(
21+
'No hub defined or ' + method + ' was not found on the hub, please open a bug report.'
22+
);
23+
}
24+
/**
25+
* Captures an exception event and sends it to Sentry.
26+
*
27+
* @param exception An exception-like object.
28+
* @returns The generated eventId.
29+
*/
30+
function captureException(exception) {
31+
let syntheticException;
32+
try {
33+
throw new Error('Sentry syntheticException');
34+
} catch (error) {
35+
syntheticException = error;
36+
}
37+
return callOnHub('captureException', exception, {
38+
originalException: exception,
39+
syntheticException,
40+
});
41+
}
42+
exports.captureException = captureException;
43+
/**
44+
* Captures a message event and sends it to Sentry.
45+
*
46+
* @param message The message to send to Sentry.
47+
* @param level Define the level of the message.
48+
* @returns The generated eventId.
49+
*/
50+
function captureMessage(message, level) {
51+
let syntheticException;
52+
try {
53+
throw new Error(message);
54+
} catch (exception) {
55+
syntheticException = exception;
56+
}
57+
return callOnHub('captureMessage', message, level, {
58+
originalException: message,
59+
syntheticException,
60+
});
61+
}
62+
exports.captureMessage = captureMessage;
63+
/**
64+
* Captures a manually created event and sends it to Sentry.
65+
*
66+
* @param event The event to send to Sentry.
67+
* @returns The generated eventId.
68+
*/
69+
function captureEvent(event) {
70+
return callOnHub('captureEvent', event);
71+
}
72+
exports.captureEvent = captureEvent;
73+
/**
74+
* Records a new breadcrumb which will be attached to future events.
75+
*
76+
* Breadcrumbs will be added to subsequent events to provide more context on
77+
* user's actions prior to an error or crash.
78+
*
79+
* @param breadcrumb The breadcrumb to record.
80+
*/
81+
function addBreadcrumb(breadcrumb) {
82+
callOnHub('addBreadcrumb', breadcrumb);
83+
}
84+
exports.addBreadcrumb = addBreadcrumb;
85+
/**
86+
* Callback to set context information onto the scope.
87+
* @param callback Callback function that receives Scope.
88+
*/
89+
function configureScope(callback) {
90+
callOnHub('configureScope', callback);
91+
}
92+
exports.configureScope = configureScope;
93+
/**
94+
* Creates a new scope with and executes the given operation within.
95+
* The scope is automatically removed once the operation
96+
* finishes or throws.
97+
*
98+
* This is essentially a convenience function for:
99+
*
100+
* pushScope();
101+
* callback();
102+
* popScope();
103+
*
104+
* @param callback that will be enclosed into push/popScope.
105+
*/
106+
function withScope(callback) {
107+
callOnHub('withScope', callback);
108+
}
109+
exports.withScope = withScope;
110+
/**
111+
* Calls a function on the latest client. Use this with caution, it's meant as
112+
* in "internal" helper so we don't need to expose every possible function in
113+
* the shim. It is not guaranteed that the client actually implements the
114+
* function.
115+
*
116+
* @param method The method to call on the client/client.
117+
* @param args Arguments to pass to the client/fontend.
118+
*/
119+
function _callOnClient(method) {
120+
const args = [];
121+
for (let _i = 1; _i < arguments.length; _i++) {
122+
args[_i - 1] = arguments[_i];
123+
}
124+
callOnHub.apply(void 0, tslib_1.__spread(['_invokeClient', method], args));
125+
}
126+
exports._callOnClient = _callOnClient;
127+
// # sourceMappingURL=index.js.map

fixtures/artifact_bundle_duplicated_debug_ids/files/_/_/index.js.map

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

fixtures/artifact_bundle_duplicated_debug_ids/files/_/_/index.min.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
{
2+
"debug_id": "67429b2f-1d9e-43bb-a626-771a1e37555c",
3+
"files": {
4+
"files/_/_/index.js": {
5+
"url": "~/index.js",
6+
"type": "source"
7+
},
8+
"files/_/_/index.min.js": {
9+
"url": "~/index.min.js",
10+
"type": "minified_source",
11+
"headers": {
12+
"Debug-id": "Eb6e60f1-65ff-4f6f-Adff-f1bbeDEd627b",
13+
"Sourcemap": "index.js.map"
14+
}
15+
},
16+
"files/_/_/index.js.map": {
17+
"url": "~/index.js.map",
18+
"type": "source_map",
19+
"headers": {
20+
"debug-id": "Eb6e60f1-65ff-4f6f-Adff-f1bbeDEd627b"
21+
}
22+
},
23+
"files/_/_/bundle1.js": {
24+
"url": "~/bundle1.js",
25+
"type": "source"
26+
},
27+
"files/_/_/bundle1.min.js": {
28+
"url": "~/bundle1.min.js",
29+
"type": "minified_source",
30+
"headers": {
31+
"Debug-id": "Eb6e60f1-65ff-4f6f-Adff-f1bbeDEd627b",
32+
"Sourcemap": "index.js.map"
33+
}
34+
},
35+
"files/_/_/bundle1.min.js.map": {
36+
"url": "~/bundle1.min.js.map",
37+
"type": "source_map",
38+
"headers": {
39+
"debug-id": "Eb6e60f1-65ff-4f6f-Adff-f1bbeDEd627b"
40+
}
41+
}
42+
}
43+
}

src/sentry/tasks/assemble.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import hashlib
22
import logging
33
from os import path
4-
from typing import List, Optional, Tuple
4+
from typing import List, Optional, Set, Tuple
55

66
from django.db import IntegrityError, router, transaction
77
from django.db.models import Q
@@ -255,8 +255,9 @@ def _normalize_debug_id(debug_id: Optional[str]) -> Optional[str]:
255255

256256
def _extract_debug_ids_from_manifest(
257257
manifest: dict,
258-
) -> Tuple[Optional[str], List[Tuple[SourceFileType, str]]]:
259-
debug_ids_with_types = []
258+
) -> Tuple[Optional[str], Set[Tuple[SourceFileType, str]]]:
259+
# We use a set, since we might have the same debug_id and file_type.
260+
debug_ids_with_types = set()
260261

261262
# We also want to extract the bundle_id which is also known as the bundle debug_id. This id is used to uniquely
262263
# identify a specific ArtifactBundle in case for example of future deletion.
@@ -278,7 +279,7 @@ def _extract_debug_ids_from_manifest(
278279
and file_type is not None
279280
and (source_file_type := SourceFileType.from_lowercase_key(file_type)) is not None
280281
):
281-
debug_ids_with_types.append((source_file_type, debug_id))
282+
debug_ids_with_types.add((source_file_type, debug_id))
282283

283284
return bundle_id, debug_ids_with_types
284285

tests/sentry/tasks/test_assemble.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def test_artifacts_with_debug_ids(self):
235235
for debug_id in expected_debug_ids:
236236
debug_id_artifact_bundles = DebugIdArtifactBundle.objects.filter(
237237
organization_id=self.organization.id, debug_id=debug_id
238-
)
238+
).order_by("-debug_id", "source_file_type")
239239
assert len(debug_id_artifact_bundles) == 2
240240
assert debug_id_artifact_bundles[0].artifact_bundle.file.size == len(bundle_file)
241241
# We check if the bundle to which each debug id entry is connected has the correct bundle_id.
@@ -267,6 +267,31 @@ def test_artifacts_with_debug_ids(self):
267267
ReleaseArtifactBundle.objects.all().delete()
268268
ProjectArtifactBundle.objects.all().delete()
269269

270+
def test_upload_artifacts_with_duplicated_debug_ids(self):
271+
bundle_file = self.create_artifact_bundle_zip(
272+
fixture_path="artifact_bundle_duplicated_debug_ids", project=self.project.id
273+
)
274+
blob1 = FileBlob.from_file(ContentFile(bundle_file))
275+
total_checksum = sha1(bundle_file).hexdigest()
276+
expected_debug_ids = ["eb6e60f1-65ff-4f6f-adff-f1bbeded627b"]
277+
278+
assemble_artifacts(
279+
org_id=self.organization.id,
280+
project_ids=[self.project.id],
281+
version="1.0",
282+
dist="android",
283+
checksum=total_checksum,
284+
chunks=[blob1.checksum],
285+
upload_as_artifact_bundle=True,
286+
)
287+
288+
for debug_id in expected_debug_ids:
289+
debug_id_artifact_bundles = DebugIdArtifactBundle.objects.filter(
290+
organization_id=self.organization.id, debug_id=debug_id
291+
)
292+
# We expect to have only two entries, since we have duplicated debug_id, file_type pairs.
293+
assert len(debug_id_artifact_bundles) == 2
294+
270295
def test_upload_multiple_artifact_with_same_bundle_id(self):
271296
bundle_file = self.create_artifact_bundle_zip(
272297
fixture_path="artifact_bundle_debug_ids", project=self.project.id

0 commit comments

Comments
 (0)