Skip to content

Commit 4ad89c0

Browse files
committed
Revert "feat(tus): split into separate creation and upload requests"
This temporarily reverts commit 5a4097f1db4ae749370b0d82a2dc447ce5e8fc78 until it's supported on the server side.
1 parent a9962dd commit 4ad89c0

4 files changed

Lines changed: 41 additions & 166 deletions

File tree

src/transports/sentry_http_transport.c

Lines changed: 18 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,10 @@ prepare_tus_request_common(
224224
h->key = "x-sentry-auth";
225225
h->value = sentry__dsn_get_auth_header(dsn, user_agent);
226226

227+
h = &req->headers[req->headers_len++];
228+
h->key = "content-type";
229+
h->value = sentry__string_clone(TUS_MIME);
230+
227231
h = &req->headers[req->headers_len++];
228232
h->key = "tus-resumable";
229233
h->value = sentry__string_clone("1.0.0");
@@ -236,46 +240,18 @@ prepare_tus_request_common(
236240
}
237241

238242
static sentry_prepared_http_request_t *
239-
prepare_tus_upload_request(
240-
const char *location, const sentry_path_t *path, size_t file_size)
243+
prepare_tus_request(const sentry_path_t *path, size_t file_size,
244+
const sentry_dsn_t *dsn, const char *user_agent)
241245
{
242-
if (!location || !path) {
246+
if (!path) {
243247
return NULL;
244248
}
245-
246249
sentry_prepared_http_request_t *req
247-
= SENTRY_MAKE(sentry_prepared_http_request_t);
248-
if (!req) {
249-
return NULL;
250-
}
251-
memset(req, 0, sizeof(*req));
252-
253-
req->headers = sentry_malloc(
254-
sizeof(sentry_prepared_http_header_t) * TUS_MAX_HTTP_HEADERS);
255-
if (!req->headers) {
256-
sentry_free(req);
257-
return NULL;
250+
= prepare_tus_request_common(file_size, dsn, user_agent);
251+
if (req) {
252+
req->body_path = sentry__path_clone(path);
253+
req->body_len = file_size;
258254
}
259-
req->headers_len = 0;
260-
261-
req->method = "PATCH";
262-
req->url = sentry__string_clone(location);
263-
req->body_path = sentry__path_clone(path);
264-
req->body_len = file_size;
265-
266-
sentry_prepared_http_header_t *h;
267-
h = &req->headers[req->headers_len++];
268-
h->key = "tus-resumable";
269-
h->value = sentry__string_clone("1.0.0");
270-
271-
h = &req->headers[req->headers_len++];
272-
h->key = "content-type";
273-
h->value = sentry__string_clone(TUS_MIME);
274-
275-
h = &req->headers[req->headers_len++];
276-
h->key = "upload-offset";
277-
h->value = sentry__string_clone("0");
278-
279255
return req;
280256
}
281257

@@ -330,9 +306,8 @@ tus_upload_ref(http_transport_state_t *state, const sentry_path_t *att_dir,
330306
return false;
331307
}
332308

333-
// Step 1: TUS creation request (POST, no body)
334-
sentry_prepared_http_request_t *req
335-
= prepare_tus_request_common(file_size, state->dsn, state->user_agent);
309+
sentry_prepared_http_request_t *req = prepare_tus_request(
310+
att_file, file_size, state->dsn, state->user_agent);
336311
if (!req) {
337312
sentry__path_free(att_file);
338313
return false;
@@ -363,42 +338,19 @@ tus_upload_ref(http_transport_state_t *state, const sentry_path_t *att_dir,
363338
return false;
364339
}
365340

366-
// Step 2: TUS upload request (PATCH with file body)
367-
req = prepare_tus_upload_request(resp.location, att_file, file_size);
368-
char *location = resp.location;
369-
resp.location = NULL;
370-
http_response_cleanup(&resp);
371-
372-
if (!req) {
373-
sentry__path_free(att_file);
374-
sentry_free(location);
375-
return false;
376-
}
377-
378-
memset(&resp, 0, sizeof(resp));
379-
ok = state->send_func(state->client, req, &resp);
380-
sentry__prepared_http_request_free(req);
381-
http_response_cleanup(&resp);
382-
383-
if (!ok || resp.status_code != 204) {
384-
sentry__path_free(att_file);
385-
sentry_free(location);
386-
return false;
387-
}
388-
389341
const char *ref_ct = sentry_value_as_string(
390342
sentry_value_get_by_key(entry, "content_type"));
391343
const char *att_type = sentry_value_as_string(
392344
sentry_value_get_by_key(entry, "attachment_type"));
393345
sentry_value_t att_length
394346
= sentry_value_get_by_key(entry, "attachment_length");
395347
sentry_value_incref(att_length);
396-
sentry__envelope_add_attachment_ref(send_envelope, location, filename,
348+
sentry__envelope_add_attachment_ref(send_envelope, resp.location, filename,
397349
*ref_ct ? ref_ct : NULL, *att_type ? att_type : NULL, att_length);
398350

399351
sentry__path_remove(att_file);
400352
sentry__path_free(att_file);
401-
sentry_free(location);
353+
http_response_cleanup(&resp);
402354
return true;
403355
}
404356

@@ -745,16 +697,9 @@ sentry__http_transport_get_bgworker(sentry_transport_t *transport)
745697
}
746698

747699
sentry_prepared_http_request_t *
748-
sentry__prepare_tus_create_request(
749-
size_t file_size, const sentry_dsn_t *dsn, const char *user_agent)
750-
{
751-
return prepare_tus_request_common(file_size, dsn, user_agent);
752-
}
753-
754-
sentry_prepared_http_request_t *
755-
sentry__prepare_tus_upload_request(
756-
const char *location, const sentry_path_t *path, size_t file_size)
700+
sentry__prepare_tus_request(const sentry_path_t *path, size_t file_size,
701+
const sentry_dsn_t *dsn, const char *user_agent)
757702
{
758-
return prepare_tus_upload_request(location, path, file_size);
703+
return prepare_tus_request(path, file_size, dsn, user_agent);
759704
}
760705
#endif

src/transports/sentry_http_transport.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,9 @@ void sentry__http_transport_set_shutdown_client(
5555

5656
#ifdef SENTRY_UNITTEST
5757
void *sentry__http_transport_get_bgworker(sentry_transport_t *transport);
58-
sentry_prepared_http_request_t *sentry__prepare_tus_create_request(
59-
size_t file_size, const sentry_dsn_t *dsn, const char *user_agent);
60-
sentry_prepared_http_request_t *sentry__prepare_tus_upload_request(
61-
const char *location, const sentry_path_t *path, size_t file_size);
58+
sentry_prepared_http_request_t *sentry__prepare_tus_request(
59+
const sentry_path_t *path, size_t file_size, const sentry_dsn_t *dsn,
60+
const char *user_agent);
6261
#endif
6362

6463
#endif

tests/test_integration_tus.py

Lines changed: 11 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,8 @@ def test_tus_upload_large_attachment(cmake, httpserver):
2626
{"SENTRY_BACKEND": "none", "SENTRY_TRANSPORT_COMPRESSION": "Off"},
2727
)
2828

29-
upload_uri = "/api/123456/upload/abc123def456789/"
30-
upload_qs = "length=104857600&signature=xyz"
31-
location = httpserver.url_for(upload_uri) + "?" + upload_qs
29+
location = "/api/123456/upload/abc123def456789/?length=104857600&signature=xyz"
3230

33-
# TUS creation request (POST, no body) -> 201 + Location
3431
httpserver.expect_oneshot_request(
3532
"/api/123456/upload/",
3633
headers={"tus-resumable": "1.0.0"},
@@ -40,14 +37,6 @@ def test_tus_upload_large_attachment(cmake, httpserver):
4037
headers={"Location": location},
4138
)
4239

43-
# TUS upload request (PATCH with body) -> 204
44-
httpserver.expect_oneshot_request(
45-
upload_uri,
46-
method="PATCH",
47-
headers={"tus-resumable": "1.0.0"},
48-
query_string=upload_qs,
49-
).respond_with_data("", status=204)
50-
5140
httpserver.expect_request(
5241
"/api/123456/envelope/",
5342
headers={"x-sentry-auth": auth_header},
@@ -62,35 +51,27 @@ def test_tus_upload_large_attachment(cmake, httpserver):
6251
env=env,
6352
)
6453

65-
assert len(httpserver.log) == 4
54+
assert len(httpserver.log) == 3
6655

67-
# Find the create, upload, and envelope requests
68-
create_req = None
56+
# Find the upload request and envelope requests
6957
upload_req = None
7058
envelope_reqs = []
7159
for entry in httpserver.log:
7260
req = entry[0]
73-
if req.path == "/api/123456/upload/" and req.method == "POST":
74-
create_req = req
75-
elif upload_uri in req.path and req.method == "PATCH":
61+
if "/upload/" in req.path:
7662
upload_req = req
7763
elif "/envelope/" in req.path:
7864
envelope_reqs.append(req)
7965

80-
assert create_req is not None
8166
assert upload_req is not None
8267
assert len(envelope_reqs) == 2
8368

84-
# Verify TUS creation request headers
85-
assert create_req.headers.get("tus-resumable") == "1.0.0"
86-
upload_length = create_req.headers.get("upload-length")
87-
assert upload_length is not None
88-
assert int(upload_length) == 100 * 1024 * 1024
89-
9069
# Verify TUS upload request headers
9170
assert upload_req.headers.get("tus-resumable") == "1.0.0"
9271
assert upload_req.headers.get("content-type") == "application/offset+octet-stream"
93-
assert upload_req.headers.get("upload-offset") == "0"
72+
upload_length = upload_req.headers.get("upload-length")
73+
assert upload_length is not None
74+
assert int(upload_length) == 100 * 1024 * 1024
9475

9576
# One envelope has the resolved attachment-refs, the other is the original
9677
attachment_ref = None
@@ -202,9 +183,7 @@ def test_tus_crash_restart(cmake, httpserver):
202183
att_size = os.path.getsize(os.path.join(att_dir, att_files[0]))
203184
assert att_size >= 100 * 1024 * 1024
204185

205-
upload_uri = "/api/123456/upload/abc123def456789/"
206-
upload_qs = "length=104857600&signature=xyz"
207-
location = httpserver.url_for(upload_uri) + "?" + upload_qs
186+
location = "/api/123456/upload/abc123def456789/?length=104857600&signature=xyz"
208187

209188
# Second run: restart picks up crash and uploads via TUS
210189
httpserver.expect_oneshot_request(
@@ -216,13 +195,6 @@ def test_tus_crash_restart(cmake, httpserver):
216195
headers={"Location": location},
217196
)
218197

219-
httpserver.expect_oneshot_request(
220-
upload_uri,
221-
method="PATCH",
222-
headers={"tus-resumable": "1.0.0"},
223-
query_string=upload_qs,
224-
).respond_with_data("", status=204)
225-
226198
httpserver.expect_request(
227199
"/api/123456/envelope/",
228200
headers={"x-sentry-auth": auth_header},
@@ -235,32 +207,24 @@ def test_tus_crash_restart(cmake, httpserver):
235207
env=env,
236208
)
237209

238-
assert len(httpserver.log) == 4
210+
assert len(httpserver.log) == 3
239211

240-
create_req = None
241212
upload_req = None
242213
envelope_reqs = []
243214
for entry in httpserver.log:
244215
req = entry[0]
245-
if req.path == "/api/123456/upload/" and req.method == "POST":
246-
create_req = req
247-
elif upload_uri in req.path and req.method == "PATCH":
216+
if "/upload/" in req.path:
248217
upload_req = req
249218
elif "/envelope/" in req.path:
250219
envelope_reqs.append(req)
251220

252-
assert create_req is not None
253221
assert upload_req is not None
254222
assert len(envelope_reqs) == 2
255223

256-
# Verify TUS creation request headers
257-
assert create_req.headers.get("tus-resumable") == "1.0.0"
258-
assert int(create_req.headers.get("upload-length")) == 100 * 1024 * 1024
259-
260224
# Verify TUS upload request headers
261225
assert upload_req.headers.get("tus-resumable") == "1.0.0"
262226
assert upload_req.headers.get("content-type") == "application/offset+octet-stream"
263-
assert upload_req.headers.get("upload-offset") == "0"
227+
assert int(upload_req.headers.get("upload-length")) == 100 * 1024 * 1024
264228

265229
# One envelope has the resolved attachment-refs, the other is the original
266230
attachment_ref = None

tests/unit/test_envelopes.c

Lines changed: 9 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -742,15 +742,19 @@ SENTRY_TEST(tus_request_preparation)
742742
{
743743
SENTRY_TEST_DSN_NEW_DEFAULT(dsn);
744744

745-
// Test creation request (POST, no body)
745+
const char *test_file_str = SENTRY_TEST_PATH_PREFIX "sentry_test_tus_file";
746+
sentry_path_t *test_file_path = sentry__path_from_str(test_file_str);
747+
TEST_CHECK_INT_EQUAL(
748+
sentry__path_write_buffer(test_file_path, "test-data", 9), 0);
749+
746750
sentry_prepared_http_request_t *req
747-
= sentry__prepare_tus_create_request(9, dsn, NULL);
751+
= sentry__prepare_tus_request(test_file_path, 9, dsn, NULL);
748752
TEST_CHECK(!!req);
749753
TEST_CHECK_STRING_EQUAL(req->method, "POST");
750754
TEST_CHECK_STRING_EQUAL(
751755
req->url, "https://sentry.invalid:443/api/42/upload/");
752-
TEST_CHECK(!req->body_path);
753-
TEST_CHECK_INT_EQUAL(req->body_len, 0);
756+
TEST_CHECK(!!req->body_path);
757+
TEST_CHECK_INT_EQUAL(req->body_len, 9);
754758
TEST_CHECK(!req->body);
755759

756760
bool has_tus_resumable = false;
@@ -765,52 +769,15 @@ SENTRY_TEST(tus_request_preparation)
765769
TEST_CHECK_STRING_EQUAL(req->headers[i].value, "9");
766770
has_upload_length = true;
767771
}
768-
if (strcmp(req->headers[i].key, "content-type") == 0) {
769-
has_content_type = true;
770-
}
771-
}
772-
TEST_CHECK(has_tus_resumable);
773-
TEST_CHECK(has_upload_length);
774-
TEST_CHECK(!has_content_type);
775-
776-
sentry__prepared_http_request_free(req);
777-
778-
// Test upload request (PATCH with body)
779-
const char *test_file_str = SENTRY_TEST_PATH_PREFIX "sentry_test_tus_file";
780-
sentry_path_t *test_file_path = sentry__path_from_str(test_file_str);
781-
TEST_CHECK_INT_EQUAL(
782-
sentry__path_write_buffer(test_file_path, "test-data", 9), 0);
783-
784-
const char *location = "https://sentry.invalid/api/42/upload/abc123/";
785-
req = sentry__prepare_tus_upload_request(location, test_file_path, 9);
786-
TEST_CHECK(!!req);
787-
TEST_CHECK_STRING_EQUAL(req->method, "PATCH");
788-
TEST_CHECK_STRING_EQUAL(req->url, location);
789-
TEST_CHECK(!!req->body_path);
790-
TEST_CHECK_INT_EQUAL(req->body_len, 9);
791-
TEST_CHECK(!req->body);
792-
793-
has_tus_resumable = false;
794-
has_content_type = false;
795-
bool has_upload_offset = false;
796-
for (size_t i = 0; i < req->headers_len; i++) {
797-
if (strcmp(req->headers[i].key, "tus-resumable") == 0) {
798-
TEST_CHECK_STRING_EQUAL(req->headers[i].value, "1.0.0");
799-
has_tus_resumable = true;
800-
}
801772
if (strcmp(req->headers[i].key, "content-type") == 0) {
802773
TEST_CHECK_STRING_EQUAL(
803774
req->headers[i].value, "application/offset+octet-stream");
804775
has_content_type = true;
805776
}
806-
if (strcmp(req->headers[i].key, "upload-offset") == 0) {
807-
TEST_CHECK_STRING_EQUAL(req->headers[i].value, "0");
808-
has_upload_offset = true;
809-
}
810777
}
811778
TEST_CHECK(has_tus_resumable);
779+
TEST_CHECK(has_upload_length);
812780
TEST_CHECK(has_content_type);
813-
TEST_CHECK(has_upload_offset);
814781

815782
sentry__prepared_http_request_free(req);
816783
sentry__path_remove(test_file_path);

0 commit comments

Comments
 (0)