From 1b03fcf0334b84bbfb0796bfb182fdbf64ed8b48 Mon Sep 17 00:00:00 2001 From: Chirag-S-kotian Date: Wed, 23 Oct 2024 18:12:02 +0530 Subject: [PATCH 1/3] Refactor errorHandler to handle various error scenarios with improved error messages --- sdk/src/utils/errorHandler.test.ts | 58 ++++++++++++++++++++---------- sdk/src/utils/errorHandler.ts | 11 +++--- 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/sdk/src/utils/errorHandler.test.ts b/sdk/src/utils/errorHandler.test.ts index 733a5690..9bd43571 100644 --- a/sdk/src/utils/errorHandler.test.ts +++ b/sdk/src/utils/errorHandler.test.ts @@ -29,6 +29,7 @@ describe("errorHandler", () => { beforeEach(() => { jest.clearAllMocks(); }); + describe("handleError", () => { const actionType: ActionType = ActionType.Transaction; @@ -40,39 +41,60 @@ describe("errorHandler", () => { expect(() => handleError(actionType, mockBaseError)).toThrow(`${actionType} failed with MockErrorName`); }); - it("should throw a generic error message if errorName is undefined", () => { - // Temporarily cast mockRevertedError.data to bypass TypeScript checks for testing - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (mockRevertedError.data as any).errorName = undefined; // Simulate errorName being `undefined` + it("should throw with the revert signature if errorName is undefined", () => { + (mockRevertedError.data as Partial).errorName = undefined; + mockRevertedError.signature = "myFunction(uint256)" as `0x${string}`; jest.spyOn(mockBaseError, "walk").mockImplementation((fn: (arg0: unknown) => unknown) => { return fn(mockRevertedError) ? mockRevertedError : null; }); - // This should test the code path where `errorName` is `undefined` and fallback to an empty string - expect(() => handleError(actionType, mockBaseError)).toThrow(`${actionType} failed with `); + expect(() => handleError(actionType, mockBaseError)).toThrow(`${actionType} failed with myFunction(uint256)`); }); - it("should throw a generic error message if it's an instance of BaseError but not ContractFunctionRevertedError", () => { - const mockBaseError = new BaseError("Base error"); + it("should throw 'unknown revert reason' if both errorName and signature are undefined", () => { + (mockRevertedError.data as Partial).errorName = undefined; + mockRevertedError.signature = undefined; - jest.spyOn(mockBaseError, "walk").mockImplementation(() => null); + jest.spyOn(mockBaseError, "walk").mockImplementation((fn: (arg0: unknown) => unknown) => { + return fn(mockRevertedError) ? mockRevertedError : null; + }); - expect(() => handleError(actionType, mockBaseError)).toThrow("${type} failed"); + expect(() => handleError(actionType, mockBaseError)).toThrow(`${actionType} failed with unknown revert reason`); }); - it("should throw a generic error message if error is not an instance of BaseError", () => { - const genericError = "Something went wrong"; + it("should throw with shortMessage if error is a BaseError but not ContractFunctionRevertedError", () => { + const shortMessage = "A short message"; + const mockBaseErrorWithShortMessage = new BaseError("Base error"); + mockBaseErrorWithShortMessage.shortMessage = shortMessage; - expect(() => handleError(actionType, genericError)).toThrow(`${actionType} failed with ${genericError}`); + jest.spyOn(mockBaseErrorWithShortMessage, "walk").mockImplementation(() => null); + + expect(() => handleError(actionType, mockBaseErrorWithShortMessage)).toThrow( + `${actionType} failed with ${shortMessage}`, + ); }); - it("should throw a generic error message even when there is no errorName in ContractFunctionRevertedError", () => { - jest.spyOn(mockBaseError, "walk").mockImplementation((fn: (arg0: unknown) => unknown) => { - return fn(mockRevertedError) ? mockRevertedError : null; - }); + it("should throw 'An unknown error occurred' if no shortMessage is present", () => { + const mockBaseErrorWithoutShortMessage = new BaseError("Base error"); + + jest.spyOn(mockBaseErrorWithoutShortMessage, "walk").mockImplementation(() => null); + + expect(() => handleError(actionType, mockBaseErrorWithoutShortMessage)).toThrow( + `${actionType} failed with An unknown error occurred`, + ); + }); + + it("should throw with the error message if error is a native JavaScript Error", () => { + const nativeError = new Error("Native error message"); + + expect(() => handleError(actionType, nativeError)).toThrow(`${actionType} failed with Native error message`); + }); + + it("should throw 'unknown error' if the error is not an instance of BaseError or Error", () => { + const unknownError = { message: "Some unknown error" }; // Simulate an unexpected error type - expect(() => handleError(actionType, mockBaseError)).toThrow(`${actionType} failed with `); + expect(() => handleError(actionType, unknownError)).toThrow(`${actionType} failed with an unknown error`); }); }); }); diff --git a/sdk/src/utils/errorHandler.ts b/sdk/src/utils/errorHandler.ts index f552e249..d15c770d 100644 --- a/sdk/src/utils/errorHandler.ts +++ b/sdk/src/utils/errorHandler.ts @@ -5,12 +5,15 @@ export function handleError(type: ActionType, err: unknown): never { if (err instanceof BaseError) { const revertError = err.walk((err) => err instanceof ContractFunctionRevertedError); if (revertError instanceof ContractFunctionRevertedError) { - const errorName = revertError.data?.errorName ?? ""; + const errorName = revertError.data?.errorName ?? revertError.signature ?? "unknown revert reason"; throw new Error(`${type} failed with ${errorName}`); + } else { + const shortMessage = err.shortMessage ?? "An unknown error occurred"; + throw new Error(`${type} failed with ${shortMessage}`); } + } else if (err instanceof Error) { + throw new Error(`${type} failed with ${err.message}`); } else { - throw new Error(`${type} failed with ${err}`); + throw new Error(`${type} failed with an unknown error`); } - - throw new Error("${type} failed"); } From 224aa20a97d511f36ec5ba45a07269c888fcb8bd Mon Sep 17 00:00:00 2001 From: Chirag-S-kotian Date: Wed, 23 Oct 2024 18:26:16 +0530 Subject: [PATCH 2/3] updated linting in error handler --- sdk/src/utils/errorHandler.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/utils/errorHandler.test.ts b/sdk/src/utils/errorHandler.test.ts index 9bd43571..343a035b 100644 --- a/sdk/src/utils/errorHandler.test.ts +++ b/sdk/src/utils/errorHandler.test.ts @@ -92,7 +92,7 @@ describe("errorHandler", () => { }); it("should throw 'unknown error' if the error is not an instance of BaseError or Error", () => { - const unknownError = { message: "Some unknown error" }; // Simulate an unexpected error type + const unknownError = { message: "Some unknown error" }; expect(() => handleError(actionType, unknownError)).toThrow(`${actionType} failed with an unknown error`); }); From e31fe07b29a3b131a26202db6ff610da7978a53a Mon Sep 17 00:00:00 2001 From: Chirag-S-kotian Date: Wed, 23 Oct 2024 22:59:18 +0530 Subject: [PATCH 3/3] Refactor errorHandler to handle ContractFunctionRevertedError and BaseError with improved error messaging --- sdk/src/utils/errorHandler.test.ts | 14 ++++++++------ sdk/src/utils/errorHandler.ts | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/sdk/src/utils/errorHandler.test.ts b/sdk/src/utils/errorHandler.test.ts index 343a035b..9848c3a6 100644 --- a/sdk/src/utils/errorHandler.test.ts +++ b/sdk/src/utils/errorHandler.test.ts @@ -33,12 +33,14 @@ describe("errorHandler", () => { describe("handleError", () => { const actionType: ActionType = ActionType.Transaction; - it("should throw with the revert error name if error is a ContractFunctionRevertedError", () => { - jest.spyOn(mockBaseError, "walk").mockImplementation((fn: (arg0: unknown) => unknown) => { - return fn(mockRevertedError) ? mockRevertedError : null; - }); + it("should throw 'An unknown error occurred' if no shortMessage is present", () => { + const mockBaseErrorWithoutShortMessage = new BaseError("Base error"); - expect(() => handleError(actionType, mockBaseError)).toThrow(`${actionType} failed with MockErrorName`); + jest.spyOn(mockBaseErrorWithoutShortMessage, "walk").mockImplementation(() => null); + + expect(() => handleError(actionType, mockBaseErrorWithoutShortMessage)).toThrow( + `${actionType} failed with An unknown error occurred`, + ); }); it("should throw with the revert signature if errorName is undefined", () => { @@ -81,7 +83,7 @@ describe("errorHandler", () => { jest.spyOn(mockBaseErrorWithoutShortMessage, "walk").mockImplementation(() => null); expect(() => handleError(actionType, mockBaseErrorWithoutShortMessage)).toThrow( - `${actionType} failed with An unknown error occurred`, + `${actionType} failed with An unknown error`, ); }); diff --git a/sdk/src/utils/errorHandler.ts b/sdk/src/utils/errorHandler.ts index d15c770d..cfefc31f 100644 --- a/sdk/src/utils/errorHandler.ts +++ b/sdk/src/utils/errorHandler.ts @@ -1,14 +1,24 @@ import { BaseError, ContractFunctionRevertedError } from "viem"; import { ActionType } from "./constants"; +function extractErrorName(revertError: ContractFunctionRevertedError): string { + if (revertError.data?.errorName) { + return revertError.data.errorName; + } + if (revertError.signature) { + return revertError.signature; + } + return "unknown revert error"; +} + export function handleError(type: ActionType, err: unknown): never { if (err instanceof BaseError) { const revertError = err.walk((err) => err instanceof ContractFunctionRevertedError); if (revertError instanceof ContractFunctionRevertedError) { - const errorName = revertError.data?.errorName ?? revertError.signature ?? "unknown revert reason"; + const errorName = extractErrorName(revertError); throw new Error(`${type} failed with ${errorName}`); } else { - const shortMessage = err.shortMessage ?? "An unknown error occurred"; + const shortMessage = err.shortMessage ?? "an unknown error"; throw new Error(`${type} failed with ${shortMessage}`); } } else if (err instanceof Error) {