Skip to content

Commit f7dfbe8

Browse files
authored
Merge pull request #923 from hittyt/fix/filesystem-not-found-log-noise
fix(sdk): avoid ERROR-level logs for expected file-not-found on read
2 parents 8886b07 + 29fab0b commit f7dfbe8

4 files changed

Lines changed: 184 additions & 3 deletions

File tree

sdks/sandbox/kotlin/sandbox/src/main/kotlin/com/alibaba/opensandbox/sandbox/domain/exceptions/SandboxException.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@ data class SandboxError(
180180
const val INVALID_ARGUMENT = "INVALID_ARGUMENT"
181181
const val UNEXPECTED_RESPONSE = "UNEXPECTED_RESPONSE"
182182

183+
/** The requested file or directory does not exist (server responds with HTTP 404). */
184+
const val FILE_NOT_FOUND = "FILE_NOT_FOUND"
185+
183186
/** Pool-specific: no idle sandbox and policy is FAIL_FAST. */
184187
const val POOL_EMPTY = "POOL_EMPTY"
185188

sdks/sandbox/kotlin/sandbox/src/main/kotlin/com/alibaba/opensandbox/sandbox/infrastructure/adapters/converter/ExceptionConverter.kt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,21 @@ import com.alibaba.opensandbox.sandbox.api.execd.infrastructure.ClientException
3939
import com.alibaba.opensandbox.sandbox.api.execd.infrastructure.ServerError as ExecdServerError
4040
import com.alibaba.opensandbox.sandbox.api.execd.infrastructure.ServerException as ExecdServerException
4141

42+
/**
43+
* Returns `true` when this throwable represents an expected "file or directory does not exist"
44+
* outcome rather than a genuine failure.
45+
*
46+
* Detection is intentionally restricted to the explicit [SandboxError.FILE_NOT_FOUND] server
47+
* error code rather than a bare HTTP 404. A 404 whose body cannot be parsed is mapped to
48+
* [SandboxError.UNEXPECTED_RESPONSE] and may indicate a real endpoint/routing/configuration
49+
* regression, which must stay loud (ERROR) instead of being silently downgraded.
50+
*
51+
* Callers (and the adapters themselves) use this to avoid treating a missing file as an error,
52+
* e.g. logging it at ERROR level with a full stack trace, which is just noise for a perfectly
53+
* normal control-flow case such as polling for a not-yet-created file.
54+
*/
55+
fun Throwable.isFileNotFound(): Boolean = this is SandboxApiException && error.code == SandboxError.FILE_NOT_FOUND
56+
4257
fun Exception.toSandboxException(): SandboxException {
4358
return when (this) {
4459
is SandboxException -> this

sdks/sandbox/kotlin/sandbox/src/main/kotlin/com/alibaba/opensandbox/sandbox/infrastructure/adapters/service/FilesystemAdapter.kt

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import com.alibaba.opensandbox.sandbox.infrastructure.adapters.converter.Filesys
3535
import com.alibaba.opensandbox.sandbox.infrastructure.adapters.converter.FilesystemConverter.toApiReplaceFileContentMap
3636
import com.alibaba.opensandbox.sandbox.infrastructure.adapters.converter.FilesystemConverter.toEntryInfo
3737
import com.alibaba.opensandbox.sandbox.infrastructure.adapters.converter.FilesystemConverter.toEntryInfoMap
38+
import com.alibaba.opensandbox.sandbox.infrastructure.adapters.converter.isFileNotFound
3839
import com.alibaba.opensandbox.sandbox.infrastructure.adapters.converter.parseSandboxError
3940
import com.alibaba.opensandbox.sandbox.infrastructure.adapters.converter.toSandboxException
4041
import kotlinx.serialization.json.buildJsonObject
@@ -108,7 +109,7 @@ internal class FilesystemAdapter(
108109
return response.body?.source()?.readString(charset) ?: ""
109110
}
110111
} catch (e: Exception) {
111-
logger.error("Failed to read file with encoding $encoding: $path", e)
112+
logReadFailure("Failed to read file with encoding $encoding: $path", e)
112113
throw e.toSandboxException()
113114
}
114115
}
@@ -134,7 +135,7 @@ internal class FilesystemAdapter(
134135
return response.body?.bytes() ?: ByteArray(0)
135136
}
136137
} catch (e: Exception) {
137-
logger.error("Failed to read file as byte array: $path", e)
138+
logReadFailure("Failed to read file as byte array: $path", e)
138139
throw e.toSandboxException()
139140
}
140141
}
@@ -167,7 +168,7 @@ internal class FilesystemAdapter(
167168
return response.body?.byteStream()
168169
?: throw IllegalStateException("Response body is null")
169170
} catch (e: Exception) {
170-
logger.error("Failed to read file as stream: $path", e)
171+
logReadFailure("Failed to read file as stream: $path", e)
171172
throw e.toSandboxException()
172173
}
173174
}
@@ -335,6 +336,26 @@ internal class FilesystemAdapter(
335336
}
336337
}
337338

339+
/**
340+
* Logs a failed read operation, distinguishing genuine failures from the expected
341+
* "file does not exist" case.
342+
*
343+
* A missing file is a normal control-flow outcome (e.g. polling for a not-yet-created
344+
* file), so it is logged at DEBUG level instead of ERROR to avoid flooding callers'
345+
* error logs and monitoring with stack traces for a non-error condition. The exception
346+
* is still propagated to the caller unchanged.
347+
*/
348+
private fun logReadFailure(
349+
message: String,
350+
e: Exception,
351+
) {
352+
if (e.isFileNotFound()) {
353+
logger.debug(message, e)
354+
} else {
355+
logger.error(message, e)
356+
}
357+
}
358+
338359
private fun getCharsetFromEncoding(encoding: String): Charset {
339360
try {
340361
return charset(encoding)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
* Copyright 2025 Alibaba Group Holding Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.alibaba.opensandbox.sandbox.infrastructure.adapters.service
18+
19+
import com.alibaba.opensandbox.sandbox.HttpClientProvider
20+
import com.alibaba.opensandbox.sandbox.config.ConnectionConfig
21+
import com.alibaba.opensandbox.sandbox.domain.exceptions.SandboxApiException
22+
import com.alibaba.opensandbox.sandbox.domain.exceptions.SandboxError
23+
import com.alibaba.opensandbox.sandbox.domain.models.sandboxes.SandboxEndpoint
24+
import com.alibaba.opensandbox.sandbox.infrastructure.adapters.converter.isFileNotFound
25+
import okhttp3.mockwebserver.MockResponse
26+
import okhttp3.mockwebserver.MockWebServer
27+
import org.junit.jupiter.api.AfterEach
28+
import org.junit.jupiter.api.Assertions.assertEquals
29+
import org.junit.jupiter.api.Assertions.assertFalse
30+
import org.junit.jupiter.api.Assertions.assertTrue
31+
import org.junit.jupiter.api.BeforeEach
32+
import org.junit.jupiter.api.Test
33+
import org.junit.jupiter.api.assertThrows
34+
35+
class FilesystemAdapterTest {
36+
private lateinit var mockWebServer: MockWebServer
37+
private lateinit var filesystemAdapter: FilesystemAdapter
38+
private lateinit var httpClientProvider: HttpClientProvider
39+
40+
@BeforeEach
41+
fun setUp() {
42+
mockWebServer = MockWebServer()
43+
mockWebServer.start()
44+
45+
val host = mockWebServer.hostName
46+
val port = mockWebServer.port
47+
val endpoint = SandboxEndpoint("$host:$port")
48+
49+
val config =
50+
ConnectionConfig.builder()
51+
.domain("$host:$port")
52+
.protocol("http")
53+
.build()
54+
55+
httpClientProvider = HttpClientProvider(config)
56+
filesystemAdapter = FilesystemAdapter(httpClientProvider, endpoint)
57+
}
58+
59+
@AfterEach
60+
fun tearDown() {
61+
mockWebServer.shutdown()
62+
httpClientProvider.close()
63+
}
64+
65+
@Test
66+
fun `readFile surfaces FILE_NOT_FOUND error code on 404 so callers can distinguish it`() {
67+
mockWebServer.enqueue(
68+
MockResponse()
69+
.setResponseCode(404)
70+
.setBody(
71+
"""{"code":"FILE_NOT_FOUND","message":"file not found. open /tmp/missing.txt: no such file or directory"}""",
72+
),
73+
)
74+
75+
val exception =
76+
assertThrows<SandboxApiException> {
77+
filesystemAdapter.readFile("/tmp/missing.txt", "UTF-8", null)
78+
}
79+
80+
assertEquals(404, exception.statusCode)
81+
assertEquals(SandboxError.FILE_NOT_FOUND, exception.error.code)
82+
// The exception itself is recognised as a "not found" condition, which is what the
83+
// adapter relies on to avoid emitting ERROR-level log noise for an expected outcome.
84+
assertTrue(exception.isFileNotFound())
85+
}
86+
87+
@Test
88+
fun `readFile returns content on success`() {
89+
mockWebServer.enqueue(
90+
MockResponse()
91+
.setResponseCode(200)
92+
.setBody("hello world"),
93+
)
94+
95+
val content = filesystemAdapter.readFile("/tmp/hello.txt", "UTF-8", null)
96+
97+
assertEquals("hello world", content)
98+
}
99+
100+
@Test
101+
fun `isFileNotFound is true for FILE_NOT_FOUND error code`() {
102+
val exception =
103+
SandboxApiException(
104+
message = "Failed to read file. Status code: 404",
105+
statusCode = 404,
106+
error = SandboxError(SandboxError.FILE_NOT_FOUND),
107+
)
108+
109+
assertTrue(exception.isFileNotFound())
110+
}
111+
112+
@Test
113+
fun `isFileNotFound is false for other API errors`() {
114+
val exception =
115+
SandboxApiException(
116+
message = "Internal server error",
117+
statusCode = 500,
118+
error = SandboxError(SandboxError.UNEXPECTED_RESPONSE),
119+
)
120+
121+
assertFalse(exception.isFileNotFound())
122+
}
123+
124+
@Test
125+
fun `isFileNotFound is false for a 404 without an explicit FILE_NOT_FOUND code`() {
126+
// A 404 whose body could not be parsed is mapped to UNEXPECTED_RESPONSE. It may indicate a
127+
// real endpoint/routing regression, so it must NOT be downgraded to a not-found condition.
128+
val exception =
129+
SandboxApiException(
130+
message = "Failed to read file. Status code: 404",
131+
statusCode = 404,
132+
error = SandboxError(SandboxError.UNEXPECTED_RESPONSE),
133+
)
134+
135+
assertFalse(exception.isFileNotFound())
136+
}
137+
138+
@Test
139+
fun `isFileNotFound is false for non-sandbox exceptions`() {
140+
assertFalse(RuntimeException("boom").isFileNotFound())
141+
}
142+
}

0 commit comments

Comments
 (0)