mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 11:48:24 +00:00
fix(blossom): go to the origin when the local cache answers without the blob
The bridge only fell back when the connection was refused. A cache that is up and simply does not hold the blob answers 404 — the ordinary state of a cache — and that 404 went straight back to the caller, so the image, video or encrypted file failed to load. Nothing retried the origin, and OkHttp then cached the 404, so the media stayed broken afterwards even once the cache could serve it. Found while testing NIP-17 encrypted media against a local cache that had never seen the blob: the message rendered as a bare `.bin` link. The encrypted path itself was fine — the key is registered under the rewritten URL and the blob decrypts (verified on device: `decrypt OK in=980 out=964`) — it was the miss that was fatal, and it would have been equally fatal for any feed image. A miss must never be worse than having no cache at all, so any unsuccessful response from the cache now closes and re-asks the origin. It is deliberately not reported through onUnreachable: the cache answered, so it is alive and the bridge should stay on. Verified on the tablet: with the cache holding nothing, the encrypted image in a DM renders again, loaded from the origin after the 404. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b32e9e163c
commit
a39e09b470
+25
-11
@@ -92,17 +92,31 @@ class LocalBlossomCacheRedirectInterceptor(
|
||||
|
||||
keyCache?.get(request.url.toString())?.let { keyCache.add(rewritten.toString(), it) }
|
||||
|
||||
return try {
|
||||
chain.proceed(
|
||||
request
|
||||
.newBuilder()
|
||||
.url(rewritten)
|
||||
.build(),
|
||||
)
|
||||
} catch (e: ConnectException) {
|
||||
onUnreachable()
|
||||
chain.proceed(request)
|
||||
}
|
||||
val bridged =
|
||||
try {
|
||||
chain.proceed(
|
||||
request
|
||||
.newBuilder()
|
||||
.url(rewritten)
|
||||
.build(),
|
||||
)
|
||||
} catch (e: ConnectException) {
|
||||
onUnreachable()
|
||||
return chain.proceed(request)
|
||||
}
|
||||
|
||||
if (bridged.isSuccessful) return bridged
|
||||
|
||||
// The cache answered, but not with the blob: it does not hold it and could not fetch it
|
||||
// from `xs` either (not every cache implements that, and the one that does can be offline,
|
||||
// still warming, or rate-limited). A miss is the ordinary state of a cache and must never
|
||||
// be worse than having no cache at all, so the origin is asked directly — without this the
|
||||
// 404 reached the caller and the image, video or encrypted file simply failed to load.
|
||||
//
|
||||
// Not reported through [onUnreachable]: the cache is alive and answering, so switching the
|
||||
// bridge off would be the wrong conclusion.
|
||||
bridged.close()
|
||||
return chain.proceed(request)
|
||||
}
|
||||
|
||||
private fun isLocalCache(url: HttpUrl): Boolean = url.port == LOCAL_CACHE_PORT && (url.host == LOCAL_CACHE_HOST || url.host.equals("localhost", ignoreCase = true))
|
||||
|
||||
+51
-2
@@ -62,6 +62,7 @@ class LocalBlossomCacheRedirectSafetyTest {
|
||||
private fun chain(
|
||||
request: Request,
|
||||
sent: MutableList<Request>,
|
||||
statusFor: (Request) -> Int = { 200 },
|
||||
refuse: (Request) -> Boolean = { false },
|
||||
): Interceptor.Chain =
|
||||
Proxy.newProxyInstance(
|
||||
@@ -74,12 +75,13 @@ class LocalBlossomCacheRedirectSafetyTest {
|
||||
val proceeded = args[0] as Request
|
||||
sent.add(proceeded)
|
||||
if (refuse(proceeded)) throw ConnectException("Connection refused")
|
||||
val status = statusFor(proceeded)
|
||||
Response
|
||||
.Builder()
|
||||
.request(proceeded)
|
||||
.protocol(Protocol.HTTP_1_1)
|
||||
.code(200)
|
||||
.message("OK")
|
||||
.code(status)
|
||||
.message(if (status == 200) "OK" else "Not Found")
|
||||
.body("".toResponseBody(null))
|
||||
.build()
|
||||
}
|
||||
@@ -243,4 +245,51 @@ class LocalBlossomCacheRedirectSafetyTest {
|
||||
assertEquals(1, reports)
|
||||
assertTrue(sent.single().url.host == "127.0.0.1")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aCacheMissFallsBackToTheOrigin() {
|
||||
var reports = 0
|
||||
val sent = mutableListOf<Request>()
|
||||
|
||||
val response =
|
||||
LocalBlossomCacheRedirectInterceptor(onUnreachable = { reports++ }) { true }
|
||||
.intercept(
|
||||
chain(media().build(), sent, statusFor = { if (it.url.host == "127.0.0.1") 404 else 200 }),
|
||||
)
|
||||
|
||||
// The blob still arrives: the cache is an optimisation, never a gate.
|
||||
assertEquals(200, response.code)
|
||||
assertEquals(origin, response.request.url.toString())
|
||||
assertEquals(listOf(bridged, origin), sent.map { it.url.toString() })
|
||||
// The cache answered, so it is alive — the bridge must not be switched off.
|
||||
assertEquals(0, reports)
|
||||
response.close()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aCacheErrorFallsBackToTheOriginToo() {
|
||||
val sent = mutableListOf<Request>()
|
||||
|
||||
val response =
|
||||
LocalBlossomCacheRedirectInterceptor { true }
|
||||
.intercept(
|
||||
chain(media().build(), sent, statusFor = { if (it.url.host == "127.0.0.1") 500 else 200 }),
|
||||
)
|
||||
|
||||
assertEquals(200, response.code)
|
||||
assertEquals(listOf(bridged, origin), sent.map { it.url.toString() })
|
||||
response.close()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aServedBlobIsNotRefetchedFromTheOrigin() {
|
||||
val sent = mutableListOf<Request>()
|
||||
|
||||
val response = LocalBlossomCacheRedirectInterceptor { true }.intercept(chain(media().build(), sent))
|
||||
|
||||
assertEquals(200, response.code)
|
||||
// A hit must cost exactly one request, to the cache.
|
||||
assertEquals(listOf(bridged), sent.map { it.url.toString() })
|
||||
response.close()
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user