From 914dfca3561347765bceb99339d7f174f7a5e72d Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Wed, 12 Aug 2026 18:53:59 -0700 Subject: [PATCH] surface server diagnostics as well --- src/commands/diagnose.zig | 342 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 325 insertions(+), 17 deletions(-) diff --git a/src/commands/diagnose.zig b/src/commands/diagnose.zig index 55e7875..30e95ae 100644 --- a/src/commands/diagnose.zig +++ b/src/commands/diagnose.zig @@ -47,7 +47,8 @@ pub const meta: framework.Meta = .{ \\Reports, in order: \\ local newest cached bar, TTL state, provider, failure count \\ peers newest bar held by other symbols of the same kind - \\ server what ZFIN_SERVER offers, and whether it is ahead or behind + \\ server what ZFIN_SERVER offers, whether it is ahead or behind, and + \\ whether it refreshes this symbol at all \\ provider what the upstream provider actually has right now \\ \\Then a verdict naming where the chain breaks, and what to do about it. @@ -92,6 +93,10 @@ pub const Observed = struct { /// than routinely, so "nothing tracks this" is true but misleading - the /// remedy is `zfin projections`, not a config fix. benchmark: bool = false, + /// Does the SHARED SERVER's refresh loop include this symbol? Null when the + /// server did not say: none configured, a build predating `/diagnostics`, or + /// a failed request. Only a definite `false` is ever concluded from. + server_tracked: ?bool = null, }; pub const Verdict = enum { @@ -113,6 +118,12 @@ pub const Verdict = enum { /// The provider has a newer bar, the TTL has lapsed, and the shared server is /// also behind - so a normal run syncs the server's older copy and stops. held_by_server, + /// As `held_by_server`, plus the server does not track the symbol at all - so + /// its copy is not merely behind, nothing there will ever move it. A local + /// force-refresh fixes today; only registering the symbol on the server stops + /// it recurring. Separated from `held_by_server` because the two have + /// different remedies and only one of them is durable. + server_untracked, /// The provider has a newer bar and nothing is holding us back; a refresh /// should simply work. refreshable, @@ -131,6 +142,7 @@ pub const Verdict = enum { .not_tracked => "nothing in the fetch set asks for this symbol, so no normal run will update it - however stale it gets", .held_by_ttl => "the bar exists upstream, but the local TTL has not lapsed - nothing will fetch it until it does", .held_by_server => "the bar exists upstream; the shared cache is behind too, so a normal run syncs its older copy and stops", + .server_untracked => "the shared cache serves this symbol but never refreshes it, so it will drift behind again after any local fix - register it there to make a fix stick", .refreshable => "the bar exists upstream and nothing is holding it back", .provider_behind_peers => "peers have a newer bar but the provider has nothing newer for this symbol - the gap is upstream, not in this cache", .unknown => "no provider answer, so the chain cannot be traced past the cache", @@ -148,7 +160,7 @@ pub const Verdict = enum { pub fn action(self: Verdict) ?Action { return switch (self) { - .not_tracked, .held_by_ttl, .held_by_server, .refreshable => .{ .subcommand = "cache refresh", .takes_symbol = true }, + .not_tracked, .held_by_ttl, .held_by_server, .server_untracked, .refreshable => .{ .subcommand = "cache refresh", .takes_symbol = true }, .not_cached => .{ .subcommand = "quote", .takes_symbol = true }, .demand_fetched => .{ .subcommand = "projections", .takes_symbol = false }, .current, .provider_behind_peers, .unknown => null, @@ -187,8 +199,20 @@ pub fn classify(o: Observed) Verdict { } if (o.local_fresh) return .held_by_ttl; if (o.server) |srv| { - if (srv.lessThan(provider)) return .held_by_server; + if (srv.lessThan(provider)) { + // The server being behind is the proximate cause either way. Whether + // it can EVER catch up is the part worth separating: a tracked symbol + // will move on the server's next pass, an untracked one never will. + if (o.server_tracked) |st| { + if (!st) return .server_untracked; + } + return .held_by_server; + } } + // Deliberately NOT reporting an untracked-on-server symbol whose copy is + // current as a fault. A refresh works right now, which is what the verdict + // answers; the standing risk is carried by the `server` output line instead of + // being promoted into a blocker that does not exist yet. return .refreshable; } @@ -269,21 +293,62 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { // ── server ─────────────────────────────────────────────── if (ctx.config.server_url) |base| { - if (serverNewest(io, arena, base, ctx.config.server_api_key, symbol)) |srv| { - obs.server = srv; - try out.print("server offers {f}", .{srv}); - if (obs.local) |l| { - if (srv.lessThan(l)) { - try out.print(" - BEHIND your local copy", .{}); - } else if (l.lessThan(srv)) { - try out.print(" - ahead of your local copy", .{}); + switch (serverDiagnostics(io, arena, base, ctx.config.server_api_key, symbol)) { + .view => |v| { + obs.server = v.last_date; + obs.server_tracked = v.tracked; + if (v.last_date) |srv| { + try out.print("server offers {f}", .{srv}); + try printRelativeToLocal(out, srv, obs.local); } else { - try out.print(" - same as local", .{}); + try out.print("server nothing cached there", .{}); } - } - try out.print("\n", .{}); - } else { - try out.print("server no answer for this symbol\n", .{}); + if (v.created) |c| { + try out.print(", written {f}", .{Date.fromEpoch(c)}); + } + if (v.tracked) |t| { + try out.print(", tracked: {s}", .{if (t) "yes" else "NO"}); + } + try out.print("\n", .{}); + // The line the endpoint exists for. Said plainly, because "not + // tracked" is jargon for a state whose consequence is total. + if (v.tracked) |t| { + if (!t) try out.print(" the server will never refresh this symbol on its own\n", .{}); + } + // Its OWN peer gap, labelled as the server's rather than folded + // into the local one - different corpora, and merging them would + // invent a number neither side reported. + if (v.days_behind) |db| { + if (db > 0) { + try out.print(" and is {d}d behind its own peers there", .{db}); + // The original bug's signature in one line: stamped good, + // yet behind. Only worth saying when both hold - `fresh` + // on a current copy is just healthy. + if (v.fresh orelse false) { + try out.print(", with its TTL still unlapsed - so a normal run there will not look either", .{}); + } + try out.print("\n", .{}); + } + } + if (v.fail_count) |fc| { + if (fc > 0) try out.print(" {d} consecutive provider failure(s) recorded there\n", .{fc}); + } + }, + // Fall back to the pre-`/diagnostics` probe so this command keeps + // working against a server that has not been redeployed. + .unsupported => { + if (serverNewest(io, arena, base, ctx.config.server_api_key, symbol)) |srv| { + obs.server = srv; + try out.print("server offers {f}", .{srv}); + try printRelativeToLocal(out, srv, obs.local); + try out.print(" (no /diagnostics - intent unknown)\n", .{}); + } else { + try out.print("server no answer for this symbol\n", .{}); + } + }, + // Named, not collapsed: an auth rejection and a DNS failure send the + // operator to entirely different places. + .failed => |why| try out.print("server could not ask: {s}\n", .{why}), } } else { try out.print("server ZFIN_SERVER not set\n", .{}); @@ -335,6 +400,123 @@ pub fn run(ctx: *framework.RunCtx, parsed: ParsedArgs) !void { /// A plain GET of the server's `candles_meta`, parsed for `last_date`. Read-only /// by construction: nothing is written to the local cache, which matters because /// the normal sync path would overwrite local bytes with the server's. +/// The shared server's own account of a symbol, from `GET /:symbol/diagnostics`. +/// +/// Every field optional: an older server has no such endpoint at all, and a +/// newer one may add fields this build does not know. Absent means "the server +/// did not say", never a default value that would be indistinguishable from one +/// it did say. +pub const ServerView = struct { + /// Does the SERVER's refresh loop include this symbol? This is the field the + /// whole endpoint exists for: a symbol the server serves but never refreshes + /// looks identical to a maintained one from the outside. + tracked: ?bool = null, + /// Is the server's own copy stamped fresh by its `#!expires=`? + fresh: ?bool = null, + fail_count: ?u32 = null, + /// Calendar days the server's copy is behind its same-kind peers ON THE + /// SERVER. Not comparable to the local peer gap - different corpus. + days_behind: ?i64 = null, + last_date: ?Date = null, + peer_date: ?Date = null, + /// Unix seconds when the server wrote its copy. + created: ?i64 = null, +}; + +/// Outcome of asking the server about a symbol. A tagged union rather than +/// `?ServerView` because "this server is too old to ask" and "the request broke" +/// lead to different output, and collapsing them would report an infrastructure +/// problem as a missing feature. +pub const ServerProbe = union(enum) { + view: ServerView, + /// The endpoint is absent: a server predating it. Fall back to `candles_meta`. + unsupported, + /// Anything else. Carries the error NAME so the output can say which, rather + /// than reporting a bare "unavailable" for a DNS failure and an auth + /// rejection alike. + failed: []const u8, +}; + +/// Parse a diagnostics body. Separated from the request so the shape can be +/// tested without a server. +/// +/// Unknown fields are ignored and malformed ones are left null rather than +/// failing the parse: a newer server adding a field must not blind an older +/// client to the fields it does understand. +pub fn parseServerView(arena: std.mem.Allocator, body: []const u8) ?ServerView { + const parsed = std.json.parseFromSlice(std.json.Value, arena, body, .{}) catch return null; + const obj = switch (parsed.value) { + .object => |o| o, + else => return null, + }; + var v = ServerView{}; + if (obj.get("tracked")) |x| if (x == .bool) { + v.tracked = x.bool; + }; + if (obj.get("fresh")) |x| if (x == .bool) { + v.fresh = x.bool; + }; + if (obj.get("fail_count")) |x| if (x == .integer and x.integer >= 0) { + v.fail_count = @intCast(x.integer); + }; + if (obj.get("days_behind")) |x| if (x == .integer) { + v.days_behind = x.integer; + }; + if (obj.get("created")) |x| if (x == .integer) { + v.created = x.integer; + }; + if (obj.get("last_date")) |x| if (x == .string) { + v.last_date = Date.parse(x.string) catch null; + }; + if (obj.get("peer_date")) |x| if (x == .string) { + v.peer_date = Date.parse(x.string) catch null; + }; + return v; +} + +/// " - ahead of your local copy" and friends. Extracted because both the +/// `/diagnostics` path and the older `candles_meta` fallback print it, and two +/// copies would have drifted on the equal case. +fn printRelativeToLocal(out: *std.Io.Writer, srv: Date, local: ?Date) !void { + const l = local orelse return; + if (srv.lessThan(l)) { + try out.print(" - BEHIND your local copy", .{}); + } else if (l.lessThan(srv)) { + try out.print(" - ahead of your local copy", .{}); + } else { + try out.print(" - same as local", .{}); + } +} + +fn serverDiagnostics( + io: std.Io, + arena: std.mem.Allocator, + base: []const u8, + api_key: ?[]const u8, + symbol: []const u8, +) ServerProbe { + const url = std.fmt.allocPrint(arena, "{s}/{s}/diagnostics", .{ base, symbol }) catch |e| + return .{ .failed = @errorName(e) }; + var client = http.Client.init(io, arena); + defer client.deinit(); + var hdr: [1]std.http.Header = .{.{ .name = "", .value = "" }}; + const extra: []const std.http.Header = if (api_key) |k| blk: { + hdr[0] = .{ .name = "X-API-Key", .value = k }; + break :blk hdr[0..1]; + } else &.{}; + var resp = client.request(.GET, url, null, extra) catch |err| switch (err) { + // The one error that means "ask the old way" rather than "something is + // wrong". Every other status is a real failure and says so by name. + error.NotFound => return .unsupported, + else => return .{ .failed = @errorName(err) }, + }; + defer resp.deinit(); + return if (parseServerView(arena, resp.body)) |v| + .{ .view = v } + else + .{ .failed = "MalformedBody" }; +} + fn serverNewest( io: std.Io, arena: std.mem.Allocator, @@ -472,7 +654,12 @@ test "classify: absent tiers degrade to a named unknown, never a guess" { } test "classify: every verdict offering an action names a runnable command" { - for ([_]Verdict{ .not_cached, .current, .demand_fetched, .not_tracked, .held_by_ttl, .held_by_server, .refreshable, .provider_behind_peers, .unknown }) |v| { + // Enumerated from the type, NOT a hand-written list. The hand-written version + // of this claimed to cover "every verdict" and silently skipped + // `server_untracked` the moment it was added - a test that quietly stops + // covering new cases is worse than no test, because the green tick is read as + // coverage. + for (std.enums.values(Verdict)) |v| { try testing.expect(v.summary().len > 0); // A subcommand, not a full command line - the caller appends the symbol, // so this must never already contain one. @@ -596,3 +783,124 @@ test "classify: a current benchmark is still just current" { .benchmark = true, })); } + +test "parseServerView: the live production record for SPCX" { + // Verbatim from `GET /SPCX/diagnostics` against the deployed server. The + // combination is the one this endpoint was built to surface: served, behind, + // and refreshed by nothing. + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const body = + \\{"symbol":"SPCX","tracked":false,"fresh":false,"fail_count":0,"days_behind":44,"last_date":"2026-06-29","peer_date":"2026-08-12","created":1782773387} + ; + const v = parseServerView(arena.allocator(), body).?; + try testing.expectEqual(false, v.tracked.?); + try testing.expectEqual(false, v.fresh.?); + try testing.expectEqual(@as(u32, 0), v.fail_count.?); + try testing.expectEqual(@as(i64, 44), v.days_behind.?); + try testing.expect(v.last_date.?.eql(d(2026, 6, 29))); + try testing.expect(v.peer_date.?.eql(d(2026, 8, 12))); + try testing.expectEqual(@as(i64, 1782773387), v.created.?); +} + +test "parseServerView: nulls stay null rather than becoming defaults" { + // An uncached symbol answers 200 with nulls. `tracked:false` and "the server + // did not say" must not both arrive as false - one is a finding, the other is + // an absence of information. + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const body = + \\{"symbol":"NOSUCH","tracked":false,"fresh":false,"fail_count":0,"days_behind":0,"last_date":null,"peer_date":"2026-08-12","created":null} + ; + const v = parseServerView(arena.allocator(), body).?; + try testing.expectEqual(@as(?Date, null), v.last_date); + try testing.expectEqual(@as(?i64, null), v.created); + try testing.expectEqual(false, v.tracked.?); +} + +test "parseServerView: a newer server's extra fields do not blind an older client" { + // Forward compatibility is the whole reason fields are read individually + // rather than by struct coercion: an added field must not cost the client + // every field it does understand. + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const body = + \\{"tracked":true,"unheard_of":{"nested":[1,2]},"days_behind":3,"expires":123} + ; + const v = parseServerView(arena.allocator(), body).?; + try testing.expectEqual(true, v.tracked.?); + try testing.expectEqual(@as(i64, 3), v.days_behind.?); +} + +test "parseServerView: junk yields null, and a wrong-typed field is skipped not fatal" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + // Not JSON at all - e.g. an HTML error page from a proxy. + try testing.expectEqual(@as(?ServerView, null), parseServerView(a, "502")); + // Valid JSON, wrong shape. + try testing.expectEqual(@as(?ServerView, null), parseServerView(a, "[1,2,3]")); + // A field of the wrong type is dropped; the rest still parses. Better than + // failing the whole record over one bad value. + const v = parseServerView(a, "{\"tracked\":\"yes\",\"days_behind\":7}").?; + try testing.expectEqual(@as(?bool, null), v.tracked); + try testing.expectEqual(@as(i64, 7), v.days_behind.?); + // An unparseable date is null, not an error. + const v2 = parseServerView(a, "{\"last_date\":\"not-a-date\"}").?; + try testing.expectEqual(@as(?Date, null), v2.last_date); +} + +test "classify: a server that never refreshes the symbol outranks plain held_by_server" { + // SPCX's real state. `held_by_server` is true but stops short of the part + // that matters: no pass on the server will ever move this copy, so a local + // refresh fixes today and nothing else. + const base = Observed{ + .local = d(2026, 6, 29), + .local_fresh = false, + .peer = d(2026, 8, 12), + .server = d(2026, 6, 29), + .provider = d(2026, 8, 12), + .tracked = true, + }; + var o = base; + o.server_tracked = false; + try testing.expectEqual(Verdict.server_untracked, classify(o)); + + // Tracked on the server: it will catch up on the next pass, so the weaker + // verdict is the correct one. + o.server_tracked = true; + try testing.expectEqual(Verdict.held_by_server, classify(o)); + + // The server did not say (older build, or no server): must not be read as + // untracked. Silence is not a finding. + o.server_tracked = null; + try testing.expectEqual(Verdict.held_by_server, classify(o)); +} + +test "classify: untracked on the server is not a blocker while its copy is current" { + // A refresh works right now - that is what the verdict answers. The standing + // risk that nothing will refresh it later belongs on the `server` output line, + // not promoted into a blocker that does not exist yet. + try testing.expectEqual(Verdict.refreshable, classify(.{ + .local = d(2026, 8, 6), + .local_fresh = false, + .server = d(2026, 8, 12), + .provider = d(2026, 8, 12), + .tracked = true, + .server_tracked = false, + })); +} + +test "classify: the local TTL still outranks an untracked server" { + // Ordering by proximate cause, matching `held_by_ttl` over `held_by_server`: + // an unlapsed TTL stops the fetch inside `getCandles`, before any sync is + // attempted, so the server's intent has not come into play yet. + try testing.expectEqual(Verdict.held_by_ttl, classify(.{ + .local = d(2026, 6, 29), + .local_fresh = true, + .server = d(2026, 6, 29), + .provider = d(2026, 8, 12), + .tracked = true, + .server_tracked = false, + })); +}