Improve error handling

This commit is contained in:
tdurieux
2026-05-06 18:43:36 +03:00
parent aae6eae6eb
commit da78708b7b
16 changed files with 360 additions and 49 deletions
+19 -1
View File
@@ -11,6 +11,7 @@ import StorageBase, { FILE_TYPE } from "./Storage";
import FileModel from "../model/files/files.model";
import { IFile } from "../model/files/files.types";
import { createLogger, serializeError } from "../logger";
import AnonymousError from "../AnonymousError";
const logger = createLogger("fs");
@@ -62,7 +63,9 @@ export default class FileSystem extends StorageBase {
async write(
repoId: string,
p: string,
data: string | Readable
data: string | Readable,
_source?: string,
expectedSize?: number
): Promise<void> {
const fullPath = join(config.FOLDER, this.repoPath(repoId), p);
// Atomic write: stream into a sibling .tmp and only rename into place
@@ -98,6 +101,21 @@ export default class FileSystem extends StorageBase {
data.pipe(ws);
});
}
// Size guard: stat the tmp before renaming. A short read produces a
// truncated tmp file that we MUST NOT promote — accepting it would
// shadow the real content on every subsequent request. Allow >=
// expectedSize because Git LFS files resolve to content larger than
// the pointer's recorded size.
if (typeof expectedSize === "number" && expectedSize > 0) {
const stat = await fs.promises.stat(tmpPath);
if (stat.size < expectedSize) {
await fs.promises.rm(tmpPath, { force: true }).catch(() => undefined);
throw new AnonymousError("storage_write_size_mismatch", {
httpStatus: 502,
object: { path: p, expected: expectedSize, actual: stat.size },
});
}
}
await fs.promises.rename(tmpPath, fullPath);
} catch (err) {
logger.error("write failed", serializeError(err));
+38 -3
View File
@@ -7,7 +7,7 @@ import {
import { Upload } from "@aws-sdk/lib-storage";
import { NodeHttpHandler } from "@smithy/node-http-handler";
import config from "../../config";
import { pipeline, Readable, Transform } from "stream";
import { pipeline, Readable, Transform, PassThrough } from "stream";
import ArchiveStreamToS3 from "decompress-stream-to-s3";
import { Response } from "express";
import { lookup } from "mime-types";
@@ -174,7 +174,8 @@ export default class S3Storage extends StorageBase {
repoId: string,
path: string,
data: string | Readable,
source?: string
source?: string,
expectedSize?: number
): Promise<void> {
if (!config.S3_BUCKET) throw new Error("S3_BUCKET not set");
@@ -186,10 +187,28 @@ export default class S3Storage extends StorageBase {
// recovers from any object that does end up undersized for any
// reason.
// When we know the upstream byte count, count bytes through a
// PassThrough so we can refuse to commit a short read. The Upload
// client itself doesn't expose bytesWritten, and S3 will happily
// PutObject a truncated body if the source stream ends cleanly with
// fewer bytes than expected.
let body: string | Readable = data;
let bytesWritten = -1;
if (typeof expectedSize === "number" && expectedSize > 0 && typeof data !== "string") {
bytesWritten = 0;
const counter = new PassThrough();
counter.on("data", (chunk: Buffer) => {
bytesWritten += chunk.length;
});
data.on("error", (err: Error) => counter.destroy(err));
data.pipe(counter);
body = counter;
}
const params: PutObjectCommandInput = {
Bucket: config.S3_BUCKET,
Key: join(this.repoPath(repoId), path),
Body: data,
Body: body,
ContentType: lookup(path).toString(),
};
if (source) {
@@ -203,6 +222,22 @@ export default class S3Storage extends StorageBase {
});
await parallelUploads3.done();
if (
typeof expectedSize === "number" &&
expectedSize > 0 &&
bytesWritten >= 0 &&
bytesWritten < expectedSize
) {
// The body completed cleanly but produced fewer bytes than the
// tree said — delete the truncated object so the next request
// re-fetches from GitHub instead of serving the short blob.
await this.rm(repoId, path).catch(() => undefined);
throw new AnonymousError("storage_write_size_mismatch", {
httpStatus: 502,
object: { path, expected: expectedSize, actual: bytesWritten },
});
}
}
/** @override */
+10 -1
View File
@@ -55,7 +55,16 @@ export default abstract class StorageBase {
repoId: string,
path: string,
data: string | Readable,
source?: string
source?: string,
/**
* Expected number of bytes for the source. When provided and the
* stream produces fewer bytes (a truncated upstream response, a socket
* reset that didn't surface as an error, etc.), the write is rejected
* and any partial blob is removed instead of being committed. This is
* the load-bearing guard that keeps zero-byte cache entries from
* silently shadowing real files on subsequent reads.
*/
expectedSize?: number
): Promise<void>;
/**