From fe9d65cf3e5fc9029f40e8cc1c31fc7bdb9e7bdf Mon Sep 17 00:00:00 2001 From: Muki Kiboigo Date: Wed, 3 Jun 2026 08:03:34 -0700 Subject: [PATCH] make get and evict return errorable --- src/network/cache/Cache.zig | 4 +-- src/network/cache/FsCache.zig | 62 ++++++++++++++++---------------- src/network/layer/CacheLayer.zig | 15 ++++---- 3 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/network/cache/Cache.zig b/src/network/cache/Cache.zig index 0890c5a7..685cc7bd 100644 --- a/src/network/cache/Cache.zig +++ b/src/network/cache/Cache.zig @@ -37,7 +37,7 @@ pub fn deinit(self: *Cache) void { }; } -pub fn get(self: *Cache, arena: std.mem.Allocator, req: CacheRequest) ?CachedResponse { +pub fn get(self: *Cache, arena: std.mem.Allocator, req: CacheRequest) !?CachedResponse { return switch (self.kind) { inline else => |*c| c.get(arena, req), }; @@ -49,7 +49,7 @@ pub fn put(self: *Cache, metadata: CachedMetadata, body: []const u8) !void { }; } -pub fn evict(self: *Cache, url: []const u8) void { +pub fn evict(self: *Cache, url: []const u8) !void { return switch (self.kind) { inline else => |*c| c.evict(url), }; diff --git a/src/network/cache/FsCache.zig b/src/network/cache/FsCache.zig index 69a495b8..39a0f461 100644 --- a/src/network/cache/FsCache.zig +++ b/src/network/cache/FsCache.zig @@ -136,7 +136,7 @@ pub fn deinit(self: *FsCache) void { self.dir.close(); } -pub fn get(self: *FsCache, arena: std.mem.Allocator, req: CacheRequest) ?CachedResponse { +pub fn get(self: *FsCache, arena: std.mem.Allocator, req: CacheRequest) !?CachedResponse { const hashed_key = hashKey(req.url); const cache_p = cachePath(&hashed_key); @@ -149,9 +149,7 @@ pub fn get(self: *FsCache, arena: std.mem.Allocator, req: CacheRequest) ?CachedR std.fs.File.OpenError.FileNotFound => { log.debug(.cache, "miss", .{ .url = req.url, .hash = &hashed_key, .reason = "missing" }); }, - else => |err| { - log.warn(.cache, "open file err", .{ .url = req.url, .err = err }); - }, + else => return e, } return null; }; @@ -284,7 +282,7 @@ pub fn clear(self: *FsCache) !void { } } -pub fn evict(self: *FsCache, url: []const u8) void { +pub fn evict(self: *FsCache, url: []const u8) !void { const hashed_key = hashKey(url); const cache_p = cachePath(&hashed_key); @@ -294,7 +292,7 @@ pub fn evict(self: *FsCache, url: []const u8) void { self.dir.deleteFile(&cache_p) catch |e| switch (e) { error.FileNotFound => {}, - else => log.warn(.cache, "evict failed", .{ .url = url, .err = e }), + else => return e, }; } @@ -391,7 +389,7 @@ test "FsCache: basic put and get" { const body = "hello world"; try cache.put(meta, body); - const result = cache.get( + const result = try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -441,7 +439,7 @@ test "FsCache: get expiration" { const body = "hello world"; try cache.put(meta, body); - const result = cache.get( + const result = try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -452,7 +450,7 @@ test "FsCache: get expiration" { result.data.file.file.close(); // Expired: age = 200 + 900 = 1100 >= 1000 - const stale = cache.get( + const stale = try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -494,7 +492,7 @@ test "FsCache: put override" { const body = "hello world"; try cache.put(meta, body); - const result = cache.get( + const result = try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -534,7 +532,7 @@ test "FsCache: put override" { const body = "goodbye world"; try cache.put(meta, body); - const result = cache.get( + const result = try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -568,9 +566,11 @@ test "FsCache: garbage file" { setup.tmp.cleanup(); } + const cache = &setup.cache; + const hashed_key = hashKey("https://example.com"); const cache_p = cachePath(&hashed_key); - const file = try setup.cache.kind.fs.dir.createFile(&cache_p, .{}); + const file = try cache.kind.fs.dir.createFile(&cache_p, .{}); try file.writeAll("this is not a valid cache file !@#$%"); file.close(); @@ -579,7 +579,7 @@ test "FsCache: garbage file" { try testing.expectEqual( null, - setup.cache.get(arena.allocator(), .{ + try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = 5000, .request_headers = &.{}, @@ -615,7 +615,7 @@ test "FsCache: vary hit and miss" { try cache.put(meta, "hello world"); - const result = cache.get(arena.allocator(), .{ + const result = try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{ @@ -624,7 +624,7 @@ test "FsCache: vary hit and miss" { }) orelse return error.CacheMiss; result.data.file.file.close(); - try testing.expectEqual(null, cache.get(arena.allocator(), .{ + try testing.expectEqual(null, try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{ @@ -632,13 +632,13 @@ test "FsCache: vary hit and miss" { }, })); - try testing.expectEqual(null, cache.get(arena.allocator(), .{ + try testing.expectEqual(null, try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{}, })); - const result2 = cache.get(arena.allocator(), .{ + const result2 = try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{ @@ -677,7 +677,7 @@ test "FsCache: vary multiple headers" { try cache.put(meta, "hello world"); - const result = cache.get(arena.allocator(), .{ + const result = try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{ @@ -687,7 +687,7 @@ test "FsCache: vary multiple headers" { }) orelse return error.CacheMiss; result.data.file.file.close(); - try testing.expectEqual(null, cache.get(arena.allocator(), .{ + try testing.expectEqual(null, try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = now, .request_headers = &.{ @@ -736,7 +736,7 @@ test "FsCache: clear removes all entries" { try cache.put(base_meta_b, "body b"); // Sanity check: both are cached - const r1 = cache.get( + const r1 = try cache.get( arena.allocator(), .{ .url = "https://example.com/a", @@ -747,7 +747,7 @@ test "FsCache: clear removes all entries" { try testing.expect(r1 != null); r1.?.data.file.file.close(); - const r2 = cache.get( + const r2 = try cache.get( arena.allocator(), .{ .url = "https://example.com/b", @@ -760,7 +760,7 @@ test "FsCache: clear removes all entries" { try cache.clear(); - try testing.expectEqual(null, cache.get( + try testing.expectEqual(null, try cache.get( arena.allocator(), .{ .url = "https://example.com/a", @@ -768,7 +768,7 @@ test "FsCache: clear removes all entries" { .request_headers = &.{}, }, )); - try testing.expectEqual(null, cache.get( + try testing.expectEqual(null, try cache.get( arena.allocator(), .{ .url = "https://example.com/b", @@ -808,7 +808,7 @@ test "FsCache: put after clear works" { // Should be a miss after clear try testing.expectEqual( null, - cache.get( + try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -820,7 +820,7 @@ test "FsCache: put after clear works" { // Put again after clear — should work normally try cache.put(meta, "after clear"); - const result = cache.get( + const result = try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -865,7 +865,7 @@ test "FsCache: evict removes entry" { try cache.put(meta, "hello world"); - const result = cache.get( + const result = try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -875,9 +875,9 @@ test "FsCache: evict removes entry" { ) orelse return error.CacheMiss; result.data.file.file.close(); - cache.evict("https://example.com"); + try cache.evict("https://example.com"); - try testing.expectEqual(null, cache.get( + try testing.expectEqual(null, try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -920,7 +920,7 @@ test "FsCache: renew refreshes expiry" { // Without revalidation would expire at now+1000, but clock reset to now+500 // so still fresh at now+1200 - const r1 = cache.get( + const r1 = try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -931,7 +931,7 @@ test "FsCache: renew refreshes expiry" { r1.data.file.file.close(); // Expires at now+500+1000 = now+1500 - const stale1 = cache.get( + const stale1 = try cache.get( arena.allocator(), .{ .url = "https://example.com", @@ -972,7 +972,7 @@ test "FsCache: renew preserves body" { try cache.renew(arena.allocator(), "https://example.com", now + 100); - const result = cache.get(arena.allocator(), .{ + const result = try cache.get(arena.allocator(), .{ .url = "https://example.com", .timestamp = now + 100, .request_headers = &.{}, diff --git a/src/network/layer/CacheLayer.zig b/src/network/layer/CacheLayer.zig index 93eedabd..d441d757 100644 --- a/src/network/layer/CacheLayer.zig +++ b/src/network/layer/CacheLayer.zig @@ -61,11 +61,14 @@ fn request(ptr: *anyopaque, transfer: *Transfer) anyerror!void { var iter = req.headers.iterator(); const req_header_list = try iter.collect(arena); - const cached = transfer.client.network.cache.?.get(arena, .{ - .url = req.url, - .timestamp = std.time.timestamp(), - .request_headers = req_header_list.items, - }) orelse { + const cached = try transfer.client.network.cache.?.get( + arena, + .{ + .url = req.url, + .timestamp = std.time.timestamp(), + .request_headers = req_header_list.items, + }, + ) orelse { // Cache miss: install wrappers so we can inspect the response and decide // whether to write the body into the cache when it's done. try installCacheContext(arena, transfer, null); @@ -114,7 +117,7 @@ fn request(ptr: *anyopaque, transfer: *Transfer) anyerror!void { try installCacheContext(arena, transfer, cached); } else { // If it is expired w/o validators, evict from Cache. - transfer.client.network.cache.?.evict(req.url); + try transfer.client.network.cache.?.evict(req.url); try installCacheContext(arena, transfer, null); }