Skip to content

Commit c80cd09

Browse files
jpnurmiclaude
andcommitted
ref(cache): cache envelopes only on failed HTTP send
Previously, cache_keep unconditionally copied envelopes to the cache directory on restart (during process_old_runs), regardless of whether sending succeeded or failed. Now envelopes are cached only when the HTTP send actually fails, fixing the behavior to match the intended "offline caching" use case. - Add cache_path and refcount to sentry_run_t - Extract http_send_envelope() helper to get status_code - Cache on failed send in http_send_task when cache_keep is enabled - Add cleanup_func on transport for async cache pruning on bgworker - Remove unconditional caching from process_old_runs - Update integration tests to use HTTP transport with unreachable DSN Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3354b27 commit c80cd09

File tree

13 files changed

+236
-85
lines changed

13 files changed

+236
-85
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
**Fixes**:
6+
7+
- Fix `cache_keep` to only cache envelopes when HTTP send fails, instead of unconditionally on restart. ([#1585](https://github.com/getsentry/sentry-native/pull/1585))
8+
39
## 0.13.3
410

511
**Features**:

examples/example.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -940,6 +940,10 @@ main(int argc, char **argv)
940940
sentry_reinstall_backend();
941941
}
942942

943+
if (has_arg(argc, argv, "flush")) {
944+
sentry_flush(10000);
945+
}
946+
943947
if (has_arg(argc, argv, "sleep")) {
944948
sleep_s(10);
945949
}

include/sentry.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1476,12 +1476,13 @@ SENTRY_API int sentry_options_get_symbolize_stacktraces(
14761476
const sentry_options_t *opts);
14771477

14781478
/**
1479-
* Enables or disables storing envelopes in a persistent cache.
1479+
* Enables or disables storing envelopes that fail to send in a persistent cache.
14801480
*
1481-
* When enabled, envelopes are written to a `cache/` subdirectory within the
1482-
* database directory and retained regardless of send success or failure.
1483-
* The cache is cleared on startup based on the cache_max_items, cache_max_size,
1484-
* and cache_max_age options.
1481+
* When enabled, envelopes that fail to send are written to a `cache/`
1482+
* subdirectory within the database directory. The cache is cleared on startup
1483+
* based on the cache_max_items, cache_max_size, and cache_max_age options.
1484+
*
1485+
* Only applicable for HTTP transports.
14851486
*
14861487
* Disabled by default.
14871488
*/

src/backends/sentry_backend_crashpad.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -565,11 +565,9 @@ process_completed_reports(
565565

566566
SENTRY_DEBUGF("caching %zu completed reports", reports.size());
567567

568-
sentry_path_t *cache_dir
569-
= sentry__path_join_str(options->database_path, "cache");
570-
if (!cache_dir || sentry__path_create_dir_all(cache_dir) != 0) {
568+
sentry_path_t *cache_dir = options->run->cache_path;
569+
if (sentry__path_create_dir_all(cache_dir) != 0) {
571570
SENTRY_WARN("failed to create cache dir");
572-
sentry__path_free(cache_dir);
573571
return;
574572
}
575573

@@ -593,8 +591,6 @@ process_completed_reports(
593591
sentry__path_free(out_path);
594592
sentry_envelope_free(envelope);
595593
}
596-
597-
sentry__path_free(cache_dir);
598594
}
599595

600596
static int

src/sentry_core.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,9 @@ sentry_init(sentry_options_t *options)
293293
}
294294

295295
if (options->cache_keep) {
296-
sentry__cleanup_cache(options);
296+
if (!sentry__transport_submit_cleanup(options->transport, options)) {
297+
sentry__cleanup_cache(options);
298+
}
297299
}
298300

299301
if (options->auto_session_tracking) {

src/sentry_database.c

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
#include "sentry_json.h"
55
#include "sentry_options.h"
66
#include "sentry_session.h"
7+
#include "sentry_sync.h"
8+
#include "sentry_transport.h"
9+
#include "sentry_utils.h"
710
#include "sentry_uuid.h"
811
#include <errno.h>
912
#include <stdlib.h>
@@ -50,19 +53,32 @@ sentry__run_new(const sentry_path_t *database_path)
5053
return NULL;
5154
}
5255

56+
// `<db>/cache`
57+
sentry_path_t *cache_path = sentry__path_join_str(database_path, "cache");
58+
if (!cache_path) {
59+
sentry__path_free(run_path);
60+
sentry__path_free(lock_path);
61+
sentry__path_free(session_path);
62+
sentry__path_free(external_path);
63+
return NULL;
64+
}
65+
5366
sentry_run_t *run = SENTRY_MAKE(sentry_run_t);
5467
if (!run) {
5568
sentry__path_free(run_path);
5669
sentry__path_free(session_path);
5770
sentry__path_free(lock_path);
5871
sentry__path_free(external_path);
72+
sentry__path_free(cache_path);
5973
return NULL;
6074
}
6175

76+
run->refcount = 1;
6277
run->uuid = uuid;
6378
run->run_path = run_path;
6479
run->session_path = session_path;
6580
run->external_path = external_path;
81+
run->cache_path = cache_path;
6682
run->lock = sentry__filelock_new(lock_path);
6783
if (!run->lock) {
6884
goto error;
@@ -80,6 +96,15 @@ sentry__run_new(const sentry_path_t *database_path)
8096
return NULL;
8197
}
8298

99+
sentry_run_t *
100+
sentry__run_incref(sentry_run_t *run)
101+
{
102+
if (run) {
103+
sentry__atomic_fetch_and_add(&run->refcount, 1);
104+
}
105+
return run;
106+
}
107+
83108
void
84109
sentry__run_clean(sentry_run_t *run)
85110
{
@@ -90,12 +115,13 @@ sentry__run_clean(sentry_run_t *run)
90115
void
91116
sentry__run_free(sentry_run_t *run)
92117
{
93-
if (!run) {
118+
if (!run || sentry__atomic_fetch_and_add(&run->refcount, -1) != 1) {
94119
return;
95120
}
96121
sentry__path_free(run->run_path);
97122
sentry__path_free(run->session_path);
98123
sentry__path_free(run->external_path);
124+
sentry__path_free(run->cache_path);
99125
sentry__filelock_free(run->lock);
100126
sentry_free(run);
101127
}
@@ -151,6 +177,18 @@ sentry__run_write_external(
151177
return write_envelope(run->external_path, envelope);
152178
}
153179

180+
bool
181+
sentry__run_write_cache(
182+
const sentry_run_t *run, const sentry_envelope_t *envelope)
183+
{
184+
if (sentry__path_create_dir_all(run->cache_path) != 0) {
185+
SENTRY_ERRORF("mkdir failed: \"%s\"", run->cache_path->path);
186+
return false;
187+
}
188+
189+
return write_envelope(run->cache_path, envelope);
190+
}
191+
154192
bool
155193
sentry__run_write_session(
156194
const sentry_run_t *run, const sentry_session_t *session)
@@ -239,14 +277,6 @@ sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash)
239277
continue;
240278
}
241279

242-
sentry_path_t *cache_dir = NULL;
243-
if (options->cache_keep) {
244-
cache_dir = sentry__path_join_str(options->database_path, "cache");
245-
if (cache_dir) {
246-
sentry__path_create_dir_all(cache_dir);
247-
}
248-
}
249-
250280
sentry_pathiter_t *run_iter = sentry__path_iter_directory(run_dir);
251281
const sentry_path_t *file;
252282
while (run_iter && (file = sentry__pathiter_next(run_iter)) != NULL) {
@@ -291,25 +321,12 @@ sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash)
291321
} else if (sentry__path_ends_with(file, ".envelope")) {
292322
sentry_envelope_t *envelope = sentry__envelope_from_path(file);
293323
sentry__capture_envelope(options->transport, envelope);
294-
295-
if (cache_dir) {
296-
sentry_path_t *cached_file = sentry__path_join_str(
297-
cache_dir, sentry__path_filename(file));
298-
if (!cached_file
299-
|| sentry__path_rename(file, cached_file) != 0) {
300-
SENTRY_WARNF("failed to cache envelope \"%s\"",
301-
sentry__path_filename(file));
302-
}
303-
sentry__path_free(cached_file);
304-
continue;
305-
}
306324
}
307325

308326
sentry__path_remove(file);
309327
}
310328
sentry__pathiter_free(run_iter);
311329

312-
sentry__path_free(cache_dir);
313330
sentry__path_remove_all(run_dir);
314331
sentry__filelock_free(lock);
315332
}

src/sentry_database.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ typedef struct sentry_run_s {
1111
sentry_path_t *run_path;
1212
sentry_path_t *session_path;
1313
sentry_path_t *external_path;
14+
sentry_path_t *cache_path;
1415
sentry_filelock_t *lock;
16+
long refcount;
1517
} sentry_run_t;
1618

1719
/**
@@ -22,6 +24,11 @@ typedef struct sentry_run_s {
2224
*/
2325
sentry_run_t *sentry__run_new(const sentry_path_t *database_path);
2426

27+
/**
28+
* Increment the refcount and return the run pointer.
29+
*/
30+
sentry_run_t *sentry__run_incref(sentry_run_t *run);
31+
2532
/**
2633
* This will clean up all the files belonging to this run.
2734
*/
@@ -63,6 +70,13 @@ bool sentry__run_write_session(
6370
*/
6471
bool sentry__run_clear_session(const sentry_run_t *run);
6572

73+
/**
74+
* This will serialize and write the given envelope to disk into the cache
75+
* directory: `<database>/cache/<event-uuid>.envelope`
76+
*/
77+
bool sentry__run_write_cache(
78+
const sentry_run_t *run, const sentry_envelope_t *envelope);
79+
6680
/**
6781
* This function is essential to send crash reports from previous runs of the
6882
* program.

src/sentry_transport.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ struct sentry_transport_s {
1010
int (*flush_func)(uint64_t timeout, void *state);
1111
void (*free_func)(void *state);
1212
size_t (*dump_func)(sentry_run_t *run, void *state);
13+
void (*cleanup_func)(const sentry_options_t *options, void *state);
1314
void *state;
1415
bool running;
1516
};
@@ -92,7 +93,7 @@ sentry__transport_startup(
9293
int
9394
sentry__transport_flush(sentry_transport_t *transport, uint64_t timeout)
9495
{
95-
if (transport->flush_func && transport->running) {
96+
if (transport && transport->flush_func && transport->running) {
9697
SENTRY_DEBUG("flushing transport");
9798
return transport->flush_func(timeout, transport->state);
9899
}
@@ -147,3 +148,21 @@ sentry__transport_get_state(sentry_transport_t *transport)
147148
{
148149
return transport ? transport->state : NULL;
149150
}
151+
152+
void
153+
sentry__transport_set_cleanup_func(sentry_transport_t *transport,
154+
void (*cleanup_func)(const sentry_options_t *options, void *state))
155+
{
156+
transport->cleanup_func = cleanup_func;
157+
}
158+
159+
bool
160+
sentry__transport_submit_cleanup(
161+
sentry_transport_t *transport, const sentry_options_t *options)
162+
{
163+
if (transport && transport->cleanup_func && transport->running) {
164+
transport->cleanup_func(options, transport->state);
165+
return true;
166+
}
167+
return false;
168+
}

src/sentry_transport.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,22 @@ size_t sentry__transport_dump_queue(
5757

5858
void *sentry__transport_get_state(sentry_transport_t *transport);
5959

60+
/**
61+
* Sets the cleanup function of the transport.
62+
*
63+
* This function submits cache cleanup as a task on the transport's background
64+
* worker, so it runs after any pending send tasks from process_old_runs.
65+
*/
66+
void sentry__transport_set_cleanup_func(sentry_transport_t *transport,
67+
void (*cleanup_func)(const sentry_options_t *options, void *state));
68+
69+
/**
70+
* Submits cache cleanup to the transport's background worker.
71+
*
72+
* Returns true if cleanup was submitted, false if the transport does not
73+
* support async cleanup (caller should run cleanup synchronously).
74+
*/
75+
bool sentry__transport_submit_cleanup(
76+
sentry_transport_t *transport, const sentry_options_t *options);
77+
6078
#endif

0 commit comments

Comments
 (0)