What's wrong
Both object paths call store.OpenStaging(...) with no error handling:
Endpoints/ObjectRouteHandler.cs:332, the download miss path
Endpoints/ObjectRouteHandler.cs:226, the upload path
Inside, ObjectStore.OpenStaging runs Directory.CreateDirectory and FileStream.New(..., FileMode.CreateNew, ...) unguarded, so any I/O failure there escapes as a 500.
This breaks the project's own contract. StreamTee, ReadTeeStream and the README say that a failed store write only means the cache stays cold, and that this is "far better than a failed push". Once bytes are flowing that holds, but it does not hold when the staging file can't be opened in the first place. The writability probe runs only once at startup, so problems that appear later are not caught.
Failure scenario
The staging file can't be created, for example because of:
EACCES after a permissions change
- a read-only remount after an ext4 error
- inode exhaustion
- a stray file at
{root}/{upstream}/staging
On a download miss, the upstream response status and headers have already been copied when OpenStaging throws. The client gets a 500 even though upstream served the object. Every push fails the same way.
Reproduced by placing a file at {root}/github/staging in the integration fixture:
IOException: Cannot create '/gitlfscache-integration/github/staging' because a file or directory with the same name already exists.
There was 1 upstream fetch and the client received no object.
Suggested fix
Wrap OpenStaging in both handlers and catch IOException, UnauthorizedAccessException and ArgumentException. On failure:
- download: log, record a store-failure metric, and continue with
storeLocally: false, so the client still gets the bytes from upstream;
- upload: relay the body to upstream without teeing it into the store.
Acceptance: with the staging directory unwritable, downloads and uploads through the cache still succeed, a warning is logged and a metric is incremented, and a test covers each path.
What's wrong
Both object paths call
store.OpenStaging(...)with no error handling:Endpoints/ObjectRouteHandler.cs:332, the download miss pathEndpoints/ObjectRouteHandler.cs:226, the upload pathInside,
ObjectStore.OpenStagingrunsDirectory.CreateDirectoryandFileStream.New(..., FileMode.CreateNew, ...)unguarded, so any I/O failure there escapes as a 500.This breaks the project's own contract.
StreamTee,ReadTeeStreamand the README say that a failed store write only means the cache stays cold, and that this is "far better than a failed push". Once bytes are flowing that holds, but it does not hold when the staging file can't be opened in the first place. The writability probe runs only once at startup, so problems that appear later are not caught.Failure scenario
The staging file can't be created, for example because of:
EACCESafter a permissions change{root}/{upstream}/stagingOn a download miss, the upstream response status and headers have already been copied when
OpenStagingthrows. The client gets a 500 even though upstream served the object. Every push fails the same way.Reproduced by placing a file at
{root}/github/stagingin the integration fixture:There was 1 upstream fetch and the client received no object.
Suggested fix
Wrap
OpenStagingin both handlers and catchIOException,UnauthorizedAccessExceptionandArgumentException. On failure:storeLocally: false, so the client still gets the bytes from upstream;Acceptance: with the staging directory unwritable, downloads and uploads through the cache still succeed, a warning is logged and a metric is incremented, and a test covers each path.