mirror of
https://github.com/tdurieux/anonymous_github.git
synced 2026-08-15 00:10:44 +02:00
Fix streamer crash and misclassified transient GitHub errors
Add missing error handler on the anonymizer transform stream in the streamer route — without it, an upstream error tears down the pipe and the anonymizer emits an unhandled error that crashes the process (surfacing as ECONNRESET to the main server). Classify transient network errors (ReadError, ECONNRESET, ETIMEDOUT) as upstream_error/502 instead of file_not_found/404 so they are distinguishable in logs and don't cache-poison downstream. Update handleError tests to match the existing sanitization behavior that returns internal_error for non-AnonymousError instances.
This commit is contained in:
@@ -291,14 +291,23 @@ export default class GitHubStream extends GitHubBase {
|
|||||||
?.statusCode ??
|
?.statusCode ??
|
||||||
(error as { status?: number })?.status ??
|
(error as { status?: number })?.status ??
|
||||||
(error as { httpStatus?: number })?.httpStatus;
|
(error as { httpStatus?: number })?.httpStatus;
|
||||||
|
const errCode = (error as { code?: string })?.code;
|
||||||
|
const isTransient =
|
||||||
|
!httpStatus &&
|
||||||
|
(errCode === "ECONNRESET" ||
|
||||||
|
errCode === "ETIMEDOUT" ||
|
||||||
|
errCode === "ERR_BODY_PARSE_FAILURE" ||
|
||||||
|
error.name === "ReadError");
|
||||||
const code =
|
const code =
|
||||||
httpStatus === 422
|
httpStatus === 422
|
||||||
? "file_too_big"
|
? "file_too_big"
|
||||||
: httpStatus === 403
|
: httpStatus === 403
|
||||||
? "file_not_accessible"
|
? "file_not_accessible"
|
||||||
|
: isTransient
|
||||||
|
? "upstream_error"
|
||||||
: "file_not_found";
|
: "file_not_found";
|
||||||
const wrapped = new AnonymousError(code, {
|
const wrapped = new AnonymousError(code, {
|
||||||
httpStatus,
|
httpStatus: isTransient ? 502 : httpStatus,
|
||||||
cause: error as Error,
|
cause: error as Error,
|
||||||
object: filePath,
|
object: filePath,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -108,6 +108,7 @@ router.post("/", async (req: express.Request, res: express.Response) => {
|
|||||||
content
|
content
|
||||||
.on("error", handleStreamError)
|
.on("error", handleStreamError)
|
||||||
.pipe(anonymizer)
|
.pipe(anonymizer)
|
||||||
|
.on("error", handleStreamError)
|
||||||
.pipe(res)
|
.pipe(res)
|
||||||
.on("error", handleStreamError)
|
.on("error", handleStreamError)
|
||||||
.on("close", () => {
|
.on("close", () => {
|
||||||
|
|||||||
@@ -53,14 +53,14 @@ describe("route-utils.handleError", function () {
|
|||||||
});
|
});
|
||||||
handleError(err, res);
|
handleError(err, res);
|
||||||
expect(res.statusCode).to.equal(503);
|
expect(res.statusCode).to.equal(503);
|
||||||
expect(res.body).to.deep.equal({ error: "S3 down" });
|
expect(res.body).to.deep.equal({ error: "internal_error" });
|
||||||
});
|
});
|
||||||
|
|
||||||
it("maps messages containing 'not_found' to 404", function () {
|
it("maps messages containing 'not_found' to 404", function () {
|
||||||
const res = makeRes();
|
const res = makeRes();
|
||||||
handleError(new Error("repo_not_found"), res);
|
handleError(new Error("repo_not_found"), res);
|
||||||
expect(res.statusCode).to.equal(404);
|
expect(res.statusCode).to.equal(404);
|
||||||
expect(res.body).to.deep.equal({ error: "repo_not_found" });
|
expect(res.body).to.deep.equal({ error: "internal_error" });
|
||||||
});
|
});
|
||||||
|
|
||||||
it("maps messages containing '(Not Found)' (got HTTPError style) to 404", function () {
|
it("maps messages containing '(Not Found)' (got HTTPError style) to 404", function () {
|
||||||
@@ -73,21 +73,21 @@ describe("route-utils.handleError", function () {
|
|||||||
const res = makeRes();
|
const res = makeRes();
|
||||||
handleError(new Error("user_not_connected"), res);
|
handleError(new Error("user_not_connected"), res);
|
||||||
expect(res.statusCode).to.equal(401);
|
expect(res.statusCode).to.equal(401);
|
||||||
expect(res.body).to.deep.equal({ error: "user_not_connected" });
|
expect(res.body).to.deep.equal({ error: "internal_error" });
|
||||||
});
|
});
|
||||||
|
|
||||||
it("defaults to 500 when nothing matches", function () {
|
it("defaults to 500 when nothing matches", function () {
|
||||||
const res = makeRes();
|
const res = makeRes();
|
||||||
handleError(new Error("kaboom"), res);
|
handleError(new Error("kaboom"), res);
|
||||||
expect(res.statusCode).to.equal(500);
|
expect(res.statusCode).to.equal(500);
|
||||||
expect(res.body).to.deep.equal({ error: "kaboom" });
|
expect(res.body).to.deep.equal({ error: "internal_error" });
|
||||||
});
|
});
|
||||||
|
|
||||||
it("accepts a string error and stringifies it in the body", function () {
|
it("accepts a string error and stringifies it in the body", function () {
|
||||||
const res = makeRes();
|
const res = makeRes();
|
||||||
handleError("something_bad", res);
|
handleError("something_bad", res);
|
||||||
expect(res.statusCode).to.equal(500);
|
expect(res.statusCode).to.equal(500);
|
||||||
expect(res.body).to.deep.equal({ error: "something_bad" });
|
expect(res.body).to.deep.equal({ error: "internal_error" });
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does not call res when headersSent is true", function () {
|
it("does not call res when headersSent is true", function () {
|
||||||
|
|||||||
Reference in New Issue
Block a user