Fix (most) incorrect reports of custom errors

packages/contract-tests/test/errors.js

@@ -327,6 +327,30 @@ describe("Client appends errors (vmErrorsOnRPCResponse)", function () {
       }
     });
 
+    it("Reports that a custom error occurred when one did", async function () {
+      const example = await Example.new(1);
+      try {
+        await example.triggerCustomError();
+        assert.fail();
+      } catch (e) {
+        assert.include(e.reason, "Custom error");
+        assert.include(e.message, "Custom error");
+      }
+    });
+
+    it("Does not report a custom error when there is none", async function () {
+      const example = await Example.new(1);
+      //there was a bug where custom errors were incorrectly reported when
+      //a function that returns a value failed due to OOG or due to not occurring
+      //(e.g. refused in MetaMask).  This test is meant to check that case.
+      try {
+        await example.returnsInt.sendTransaction({ gas: 5 }); //deliberately too little gas
+        assert.fail();
+      } catch (e) {
+        assert.notInclude(e.message, "Custom error");
+      }
+    });
+
     it("appends original stacktrace for .calls", async function () {
       const example = await Example.new(1);
       try {

packages/contract-tests/test/sources/Example.sol

@@ -15,6 +15,8 @@ contract Example {
 
   enum ExampleEnum { ExampleZero, ExampleOne, ExampleTwo }
 
+  error ExampleError(uint, uint);
+
   constructor(uint val) {
     // Constructor revert
     require(val != 13);
@@ -110,6 +112,10 @@ contract Example {
     assert(false);
   }
 
+  function triggerCustomError() public {
+    revert ExampleError(2, 3);
+  }
+
   function triggerInvalidOpcode() public {
     assembly {
       invalid()

packages/contract/lib/reason.js

@@ -81,9 +81,22 @@ const reason = {
         return undefined;
       }
     } else {
-      //we can't reasonably handle custom errors here
-      //(but we can probably assume it is one?)
-      return "Custom error (could not decode)";
+      const bytesLength = (rawData.length - 2) / 2; //length of raw data in bytes
+      if (bytesLength % 32 === 4) {
+        //we can't reasonably handle custom errors here at present, sorry
+        return "Custom error (could not decode)";
+      } else {
+        //if the length isn't 4 mod 32, just give up and return undefined.
+        //the reason for this is that sometimes this function can accidentally get
+        //called on a return value rather than an error (because the tx ran out of
+        //gas or failed for a reason other than a revert, e.g., getting refused by
+        //the user in MetaMask), meaning the eth_call rerun will *succeed*, potentially
+        //resulting in a return value.  We don't want to attach an additional
+        //error message in that case, so we return undefined.
+        //(What if e.g. the tx is refused by the user in MetaMask, but the rerun yields
+        //a revert string...?  Well, that's a problem for another time...)
+        return undefined;
+      }
     }
   },