Skip to content

Commit e2195dc

Browse files
junk151516claude
andcommitted
fix(graph-buffer): log why an atomic publish failed instead of a successful dump
cbm_gbuf_dump_to_sqlite() emitted gbuf.dump regardless of the writer's result, so a run that published nothing still logged node and edge counts as if it had worked, and the failure reached the user only as the generic "Pipeline failed. Check repo_path exists and contains source files." The writer's temp -> final rename in publish_writer_output() returned a bare ERR_WRITE_FAILED. #1628 taught cbm_rename_replace() to translate the platform error into errno so callers could report why, and wired up the stage -> final rename in pipeline.c. This is the other rename on that path, and it is the one that runs first. Preserve errno across the cleanup unlink in the publish error branches, read it in graph_buffer.c before the profiling macro can clobber it, and report gbuf.dump_failed with the return code and that errno. No logging is added inside internal/cbm/, which has none today. Refs #1620, #2001. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012FxkVF773gDGNmBmsLnzN7
1 parent ffb5ebe commit e2195dc

3 files changed

Lines changed: 107 additions & 1 deletion

File tree

‎internal/cbm/sqlite_writer.c‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <string.h>
2727
#include <stdint.h>
2828
#include <stdbool.h>
29+
#include <errno.h>
2930

3031
#ifdef _WIN32
3132
#include <io.h>
@@ -1759,13 +1760,18 @@ static int sync_writer_output(FILE *fp) {
17591760
}
17601761

17611762
static int discard_writer_output(write_db_ctx_t *w, int rc) {
1763+
/* Cleanup runs after the failure that brought us here, and a library
1764+
* call may set errno even when it succeeds. Carry the reason across it
1765+
* so the caller can report WHY the publish failed. */
1766+
int failure_errno = errno;
17621767
if (w->fp) {
17631768
(void)fclose(w->fp);
17641769
w->fp = NULL;
17651770
}
17661771
if (w->temp_path[0]) {
17671772
(void)cbm_unlink(w->temp_path);
17681773
}
1774+
errno = failure_errno;
17691775
return rc;
17701776
}
17711777

@@ -1774,18 +1780,25 @@ static int publish_writer_output(write_db_ctx_t *w) {
17741780
return discard_writer_output(w, ERR_WRITE_FAILED);
17751781
}
17761782
if (fclose(w->fp) != 0) {
1783+
int failure_errno = errno;
17771784
w->fp = NULL;
17781785
if (w->temp_path[0]) {
17791786
(void)cbm_unlink(w->temp_path);
17801787
}
1788+
errno = failure_errno;
17811789
return ERR_WRITE_FAILED;
17821790
}
17831791
w->fp = NULL;
17841792
if (!w->temp_path[0] || !w->final_path[0]) {
17851793
return 0;
17861794
}
17871795
if (cbm_rename_replace(w->temp_path, w->final_path) != 0) {
1796+
/* cbm_rename_replace translated the platform error into errno so the
1797+
* caller can say what denied the publish (#1620). Preserve it across
1798+
* the cleanup unlink. */
1799+
int rename_errno = errno;
17881800
(void)cbm_unlink(w->temp_path);
1801+
errno = rename_errno;
17891802
return ERR_WRITE_FAILED;
17901803
}
17911804
/* Sidecars are removed only after the replacement succeeds. On POSIX,

‎src/graph_buffer/graph_buffer.c‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ enum {
3838
#include "foundation/mem.h"
3939
#include <sqlite3.h>
4040

41+
#include <errno.h>
4142
#include <stdatomic.h>
4243
#include <stdint.h> // int64_t
4344
#include <stdio.h>
@@ -1625,6 +1626,23 @@ static void log_dump_summary(int node_count, int edge_count) {
16251626
cbm_log_info("gbuf.dump", "nodes", b1, "edges", b2);
16261627
}
16271628

1629+
/* A dump that published nothing gets exactly one signature in the log.
1630+
* Without it the writer's return code is the only evidence, and it reaches
1631+
* the user as a generic pipeline failure that blames their repository
1632+
* (#1620, #2001). failure_errno is read by the caller at the point of
1633+
* failure: the value is only meaningful before the next library call. */
1634+
static void log_dump_failed(int rc, int failure_errno, int node_count, int edge_count) {
1635+
char b1[CBM_SZ_16];
1636+
char b2[CBM_SZ_16];
1637+
char b3[CBM_SZ_16];
1638+
char b4[CBM_SZ_16];
1639+
snprintf(b1, sizeof(b1), "%d", rc);
1640+
snprintf(b2, sizeof(b2), "%d", failure_errno);
1641+
snprintf(b3, sizeof(b3), "%d", node_count);
1642+
snprintf(b4, sizeof(b4), "%d", edge_count);
1643+
cbm_log_error("gbuf.dump_failed", "rc", b1, "errno", b2, "nodes", b3, "edges", b4);
1644+
}
1645+
16281646
static void free_dump_resources(char **url_paths, char **local_names, int edge_count,
16291647
CBMDumpEdge *dump_edges, CBMDumpNode *dump_nodes,
16301648
int64_t *temp_to_final) {
@@ -1710,6 +1728,7 @@ int cbm_gbuf_dump_to_sqlite(cbm_gbuf_t *gb, const char *path) {
17101728
* uninitialized budget from ever triggering the free). */
17111729
cbm_db_writer_t *w = cbm_writer_open(path);
17121730
if (!w) {
1731+
log_dump_failed(CBM_NOT_FOUND, errno, node_idx, 0);
17131732
free(src_nodes);
17141733
free(dump_nodes);
17151734
free(temp_to_final);
@@ -1759,12 +1778,19 @@ int cbm_gbuf_dump_to_sqlite(cbm_gbuf_t *gb, const char *path) {
17591778
int frc = cbm_writer_finalize(w, gb->project, gb->root_path, indexed_at, dump_nodes, node_idx,
17601779
dump_edges, edge_idx, gb->dump_vectors, gb->dump_vector_count,
17611780
gb->dump_token_vecs, gb->dump_token_vec_count);
1781+
/* Read errno before anything else can touch it: the profiling macro
1782+
* below is already one library call away from losing the reason. */
1783+
int finalize_errno = errno;
17621784
CBM_PROF_END_N("dump", "6_write_db_finalize", t_finalize, node_idx + edge_idx);
17631785
if (rc == 0) {
17641786
rc = frc;
17651787
}
17661788

1767-
log_dump_summary(node_idx, edge_idx);
1789+
if (rc != 0) {
1790+
log_dump_failed(rc, finalize_errno, node_idx, edge_idx);
1791+
} else {
1792+
log_dump_summary(node_idx, edge_idx);
1793+
}
17681794
free_dump_resources(url_paths, local_names, edge_idx, dump_edges, dump_nodes, temp_to_final);
17691795
free(src_nodes);
17701796
return rc;

‎tests/test_graph_buffer.c‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
#include "test_framework.h"
88
#include "graph_buffer/graph_buffer.h"
99
#include "store/store.h"
10+
#include "../src/foundation/compat.h"
11+
#include "foundation/compat_fs.h"
12+
#include "foundation/log.h"
13+
#include <stdio.h>
1014
#include <string.h>
1115

1216
/* ── Node operations ───────────────────────────────────────────── */
@@ -1118,6 +1122,66 @@ TEST(gbuf_flush_skips_orphan_edges) {
11181122
PASS();
11191123
}
11201124

1125+
/* ── Publish failure reporting ───────────────────────────────── */
1126+
1127+
static char g_log_capture[4096];
1128+
static CBMLogLevel g_prev_log_level;
1129+
1130+
static void capture_log_sink(const char *line) {
1131+
size_t used = strlen(g_log_capture);
1132+
size_t avail = sizeof(g_log_capture) - used;
1133+
if (avail <= 1) {
1134+
return;
1135+
}
1136+
int n = snprintf(g_log_capture + used, avail, "%s\n", line);
1137+
if (n < 0 || (size_t)n >= avail) {
1138+
g_log_capture[sizeof(g_log_capture) - 1] = '\0';
1139+
}
1140+
}
1141+
1142+
static void capture_logs_start(void) {
1143+
g_log_capture[0] = '\0';
1144+
g_prev_log_level = cbm_log_get_level();
1145+
cbm_log_set_level(CBM_LOG_DEBUG);
1146+
cbm_log_set_sink(capture_log_sink);
1147+
}
1148+
1149+
static const char *capture_logs_end(void) {
1150+
cbm_log_set_sink(NULL);
1151+
cbm_log_set_level(g_prev_log_level);
1152+
return g_log_capture;
1153+
}
1154+
1155+
/* A dump that publishes nothing has to say so, and say why. Renaming onto an
1156+
* existing directory is how test_sqlite_writer already forces the publish to
1157+
* fail; here it stands in for any host that denies the rename (#1620). */
1158+
TEST(gbuf_dump_failure_logs_reason) {
1159+
char dir[256];
1160+
snprintf(dir, sizeof(dir), "/tmp/cbm_gbuf_pub_XXXXXX");
1161+
ASSERT_NOT_NULL(cbm_mkdtemp(dir));
1162+
1163+
cbm_gbuf_t *gb = cbm_gbuf_new("test", "/tmp/repo");
1164+
ASSERT_NOT_NULL(gb);
1165+
int64_t id = cbm_gbuf_upsert_node(gb, "Function", "main", "pkg.main", "main.go", 1, 10, "{}");
1166+
ASSERT_GT(id, 0);
1167+
1168+
capture_logs_start();
1169+
int rc = cbm_gbuf_dump_to_sqlite(gb, dir);
1170+
const char *logs = capture_logs_end();
1171+
1172+
ASSERT(rc != 0);
1173+
ASSERT_NOT_NULL(strstr(logs, "gbuf.dump_failed"));
1174+
/* The reason survived the cleanup unlink. */
1175+
ASSERT_NOT_NULL(strstr(logs, "errno="));
1176+
ASSERT(strstr(logs, "errno=0 ") == NULL);
1177+
/* And the run is not also reported as a successful dump. */
1178+
ASSERT(strstr(logs, "msg=gbuf.dump ") == NULL);
1179+
1180+
cbm_gbuf_free(gb);
1181+
cbm_rmdir(dir);
1182+
PASS();
1183+
}
1184+
11211185
/* ── Suite ─────────────────────────────────────────────────────── */
11221186

11231187
SUITE(graph_buffer) {
@@ -1188,4 +1252,7 @@ SUITE(graph_buffer) {
11881252
RUN_TEST(gbuf_shared_ids_null_fallback);
11891253
RUN_TEST(gbuf_next_id_set_next_id_roundtrip);
11901254
RUN_TEST(gbuf_next_id_null_safe);
1255+
1256+
/* Publish failure reporting */
1257+
RUN_TEST(gbuf_dump_failure_logs_reason);
11911258
}

0 commit comments

Comments
 (0)