From af6f55472bc1aa606a8e601b26a73ffc1f2aac76 Mon Sep 17 00:00:00 2001 From: sanctus-mvp Date: Sat, 27 Jun 2026 18:43:55 +0000 Subject: [PATCH] fix: resolve undefined vars in HorizonError branch and add tests (#256) --- package-lock.json | 9 -- package.json | 4 - src/middleware/errorHandler.js | 3 + tests/errorHandler.test.js | 208 +++++++++++++++++++-------------- 4 files changed, 124 insertions(+), 100 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1f0416b..d69fd57 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,11 +19,7 @@ "helmet": "^7.1.0", "hpp": "^0.2.3", "morgan": "^1.10.0", -<<<<<<< Updated upstream - "node-cache": "5.1.2", -======= "node-cache": "^5.1.2", ->>>>>>> Stashed changes "toml": "^3.0.0", "ws": "^8.21.0" }, @@ -67,7 +63,6 @@ "integrity": "sha512-RgHBCvtjbOK2gXSNBNIkNoEc9qoVEtau3hj8gEqKQuL3HZAibKarWFEI3Lfm6EYKkLalOh8eSrj9b+ch9H/VBA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@babel/code-frame": "^7.29.7", "@babel/generator": "^7.29.7", @@ -1657,7 +1652,6 @@ "integrity": "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2153,7 +2147,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.10.12", "caniuse-lite": "^1.0.30001782", @@ -2894,7 +2887,6 @@ "integrity": "sha512-XoMjdBOwe/esVgEvLmNsD3IRHkm7fbKIUGvrleloJXUZgDHig2IPWNniv+GwjyJXzuNqVjlr5+4yVUZjycJwfQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -3155,7 +3147,6 @@ "resolved": "https://registry.npmjs.org/express/-/express-4.22.2.tgz", "integrity": "sha512-IuL+Elrou2ZvCFHs18/CIzy2Nzvo25nZ1/D2eIZlz7c+QUayAcYoiM2BthCjs+EBHVpjYjcuLDAiCWgeIX3X1Q==", "license": "MIT", - "peer": true, "dependencies": { "accepts": "~1.3.8", "array-flatten": "1.1.1", diff --git a/package.json b/package.json index c96a489..e7116b0 100644 --- a/package.json +++ b/package.json @@ -35,11 +35,7 @@ "helmet": "^7.1.0", "hpp": "^0.2.3", "morgan": "^1.10.0", -<<<<<<< Updated upstream - "node-cache": "5.1.2", -======= "node-cache": "^5.1.2", ->>>>>>> Stashed changes "toml": "^3.0.0", "ws": "^8.21.0" }, diff --git a/src/middleware/errorHandler.js b/src/middleware/errorHandler.js index 19ba6d5..5e089e8 100644 --- a/src/middleware/errorHandler.js +++ b/src/middleware/errorHandler.js @@ -37,6 +37,9 @@ function errorHandler(err, req, res, next) { const mappedStatus = mapHorizonErrorToStatus(resultCode); const status = mappedStatus ?? err.response.status ?? 400; + const code = resultCode; + const humanMessage = translateHorizonError(resultCode); + const message = horizonError.detail || horizonError.title || "Horizon Error"; logError(status, req, message); return res.status(status).json({ diff --git a/tests/errorHandler.test.js b/tests/errorHandler.test.js index bad9660..6ff26cf 100644 --- a/tests/errorHandler.test.js +++ b/tests/errorHandler.test.js @@ -1,143 +1,184 @@ const errorHandler = require("../src/middleware/errorHandler"); +const { translateHorizonError } = require("../src/utils/horizonErrors"); describe("ErrorHandler Middleware", () => { let req, res, next; beforeEach(() => { - req = { - method: "GET", - path: "/test", - }; - res = { - status: jest.fn().mockReturnThis(), - json: jest.fn(), - }; + req = { method: "GET", path: "/test" }; + res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; next = jest.fn(); }); describe("Horizon Errors", () => { - it("should map transaction result code tx_bad_seq to 409 status code", () => { + it("returns success:false and type HorizonError", () => { const err = { response: { status: 400, - data: { - title: "Transaction Failed", - detail: "The transaction failed due to bad sequence.", - extras: { - result_codes: { - transaction: "tx_bad_seq", - }, - }, - }, + data: { title: "Transaction Failed", detail: "Bad request." }, }, }; - errorHandler(err, req, res, next); + const body = res.json.mock.calls[0][0]; + expect(body.success).toBe(false); + expect(body.error.type).toBe("HorizonError"); + }); - expect(res.status).toHaveBeenCalledWith(409); - expect(res.json).toHaveBeenCalledWith({ - success: false, - error: { - type: "HorizonError", - title: "Transaction Failed", - detail: "The transaction failed due to bad sequence.", + it("includes title and detail from Horizon response data", () => { + const err = { + response: { status: 400, - extras: err.response.data.extras, + data: { title: "My Title", detail: "My Detail" }, }, - }); + }; + errorHandler(err, req, res, next); + const { error } = res.json.mock.calls[0][0]; + expect(error.title).toBe("My Title"); + expect(error.detail).toBe("My Detail"); }); - it("should map operation result code op_no_destination to 404 status code", () => { + it("maps tx_bad_seq result code to HTTP 409", () => { const err = { response: { status: 400, data: { title: "Transaction Failed", - detail: "The destination account was not found.", - extras: { - result_codes: { - operations: ["op_no_destination"], - }, - }, + detail: "Bad sequence.", + extras: { result_codes: { transaction: "tx_bad_seq" } }, }, }, }; - errorHandler(err, req, res, next); + expect(res.status).toHaveBeenCalledWith(409); + const { error } = res.json.mock.calls[0][0]; + expect(error.type).toBe("HorizonError"); + expect(error.title).toBe("Transaction Failed"); + expect(error.detail).toBe("Bad sequence."); + expect(error.extras).toEqual(err.response.data.extras); + expect(error.code).toBe("tx_bad_seq"); + expect(error.message).toBe(translateHorizonError("tx_bad_seq")); + }); - expect(res.status).toHaveBeenCalledWith(404); - expect(res.json).toHaveBeenCalledWith({ - success: false, - error: { - type: "HorizonError", - title: "Transaction Failed", - detail: "The destination account was not found.", + it("maps op_no_destination result code to HTTP 404", () => { + const err = { + response: { status: 400, - extras: err.response.data.extras, + data: { + title: "Transaction Failed", + detail: "Destination not found.", + extras: { result_codes: { operations: ["op_no_destination"] } }, + }, }, - }); + }; + errorHandler(err, req, res, next); + expect(res.status).toHaveBeenCalledWith(404); + const { error } = res.json.mock.calls[0][0]; + expect(error.code).toBe("op_no_destination"); + expect(error.message).toBe(translateHorizonError("op_no_destination")); }); - it("should fallback to err.response.status for unknown Horizon error codes", () => { + it("falls back to err.response.status for unknown result codes", () => { const err = { response: { status: 418, data: { title: "Teapot", - detail: "An unknown result code was returned.", - extras: { - result_codes: { - transaction: "tx_unknown_code_example", - }, - }, + detail: "Unknown code.", + extras: { result_codes: { transaction: "tx_unknown_code_example" } }, }, }, }; + errorHandler(err, req, res, next); + expect(res.status).toHaveBeenCalledWith(418); + const { error } = res.json.mock.calls[0][0]; + // unknown code → included as code but no translated message + expect(error.code).toBe("tx_unknown_code_example"); + expect(error.message).toBeUndefined(); + }); + it("falls back to err.response.status when no result codes present", () => { + const err = { + response: { + status: 402, + data: { title: "Payment Required", detail: "Some detail." }, + }, + }; errorHandler(err, req, res, next); + expect(res.status).toHaveBeenCalledWith(402); + const { error } = res.json.mock.calls[0][0]; + expect(error.extras).toBeNull(); + // no resultCode → code and message fields absent + expect(error.code).toBeUndefined(); + expect(error.message).toBeUndefined(); + }); - expect(res.status).toHaveBeenCalledWith(418); - expect(res.json).toHaveBeenCalledWith({ - success: false, - error: { - type: "HorizonError", - title: "Teapot", - detail: "An unknown result code was returned.", - status: 418, - extras: err.response.data.extras, + it("does not throw a ReferenceError (no undefined variable crash)", () => { + const err = { + response: { + status: 400, + data: { title: "Error", detail: "Detail." }, }, - }); + }; + expect(() => errorHandler(err, req, res, next)).not.toThrow(); }); - it("should fallback to err.response.status when no result codes are present", () => { + it("omits code and message when resultCode is null (no extras)", () => { + const err = { + response: { status: 400, data: { title: "Error", detail: "Detail." } }, + }; + errorHandler(err, req, res, next); + const { error } = res.json.mock.calls[0][0]; + expect(Object.keys(error)).not.toContain("code"); + expect(Object.keys(error)).not.toContain("message"); + }); + + it("includes code but omits message for unrecognised result code", () => { const err = { response: { - status: 402, + status: 400, data: { - title: "Payment Required", - detail: "Horizon responded with payment required.", + title: "Error", + detail: "Detail.", + extras: { result_codes: { transaction: "tx_not_in_map" } }, }, }, }; - errorHandler(err, req, res, next); + const { error } = res.json.mock.calls[0][0]; + expect(error.code).toBe("tx_not_in_map"); + expect(Object.keys(error)).not.toContain("message"); + }); - expect(res.status).toHaveBeenCalledWith(402); - expect(res.json).toHaveBeenCalledWith({ - success: false, - error: { - type: "HorizonError", - title: "Payment Required", - detail: "Horizon responded with payment required.", - status: 402, - extras: null, + it("includes translated message for recognised tx code tx_bad_auth", () => { + const err = { + response: { + status: 400, + data: { + title: "Transaction Failed", + detail: "Bad auth.", + extras: { result_codes: { transaction: "tx_bad_auth" } }, + }, }, - }); + }; + errorHandler(err, req, res, next); + const { error } = res.json.mock.calls[0][0]; + expect(error.message).toBe(translateHorizonError("tx_bad_auth")); + }); + }); + + describe("Payload Too Large Errors", () => { + it("returns 413 with PayloadTooLargeError type for entity.too.large", () => { + const err = { type: "entity.too.large" }; + errorHandler(err, req, res, next); + expect(res.status).toHaveBeenCalledWith(413); + const body = res.json.mock.calls[0][0]; + expect(body.success).toBe(false); + expect(body.error.type).toBe("PayloadTooLargeError"); }); }); describe("Validation Errors", () => { - it("should handle custom validation errors with a 400 status code", () => { + it("returns 400 with ValidationError type", () => { const err = { isValidation: true, message: "Invalid Account ID format", @@ -145,9 +186,7 @@ describe("ErrorHandler Middleware", () => { receivedValue: "G12345", expectedFormat: "G... public key", }; - errorHandler(err, req, res, next); - expect(res.status).toHaveBeenCalledWith(400); expect(res.json).toHaveBeenCalledWith({ success: false, @@ -163,18 +202,13 @@ describe("ErrorHandler Middleware", () => { }); describe("Generic Errors", () => { - it("should handle generic ServerError with a 500 status code", () => { + it("returns 500 with ServerError type", () => { const err = new Error("Database connection failed"); - errorHandler(err, req, res, next); - expect(res.status).toHaveBeenCalledWith(500); expect(res.json).toHaveBeenCalledWith({ success: false, - error: { - type: "ServerError", - message: "Database connection failed", - }, + error: { type: "ServerError", message: "Database connection failed" }, }); }); });