fix: do not cache io errors

[matklad]

When we compile & cache the contract, there are two very different kinds
of "errors" we can have:

• IO error due to the database being corrupted and such
• Contract failed to compile because it contained invalid WASM

The second outcome is normal, expected mode of execution and should be
cached. The first error is a genuine exceptional situation, and should
not be cached (eg, retrying the operation might succeed).

Test Plan

Existing tests to verify that the happy path still works. I don't believe we current have fault injection tests to make sure genuine IO-errors are handled correctly.

[EgorKulikov]

Always a pleasure to review

[bowenwang1996]

hmm isn't this equivalent to Err(_) => ()?

[matklad]

not really, the type of () is (), the type of match never {} is !. The result type here is V, and match never {} is compatible with that.

For context: Infallible is an empty enum: enum Infallible {}

[pmnoxx]

Why not use:

diff --git i/utils/near-cache/src/lib.rs w/utils/near-cache/src/lib.rs
index 069662c90..c13416021 100644
--- i/utils/near-cache/src/lib.rs
+++ w/utils/near-cache/src/lib.rs
@@ -26,10 +26,7 @@ where
         V: Clone,
         F: FnOnce(&K) -> V,
     {
-        match self.get_or_try_put(key, |k| Ok::<V, std::convert::Infallible>(f(k))) {
-            Ok(it) => it,
-            Err(never) => match never {},
-        }
+        self.get_or_try_put(key, |k| Ok::<V, std::convert::Infallible>(f(k))).unwrap()
     }
 
     /// Returns the value of they key in the cache if present, otherwise

[matklad]

.unwrap() is an operation that can in general fail at runtime. In this case it can't actually fail, but you need to read the context to understand this. In contrast, match never {} can't fail in a statically-checked way.

Really, thish should be just Result::into_ok (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.into_ok) but that needs to wait until ! is stabilized properly.

[nagisa]

In previous codebases I used

Result::<_, Infallible>::unwrap(result)

to very signal clearly enough that “yeah this is unwrap, but no it is actually into_ok”. At codegen level LLVM can see easily enough that panic within unwrap is not reachable panic won't actually materialize.

---

match never {} seems to have confused sufficient number of people that I think its probably not worth whatever static benefits it provides.

[matklad]

Excellent suggestion, thanks!

[pmnoxx]

LGTM, though, it would be nice to have a test for it..

[matklad]

LGTM, though, it would be nice to have a test for it..

That's a very good call. I've written that "we don't have fault injection tests", but of course we can cheaply add them here, as the cache is already polymorphic. And, surely, doing so uncovered a minor preexisting bug where I mixed up read error and write error!

[Longarithm]

Suggested change

	/// If the key is already in the cache, it gets moved to the head of the LDU
	/// If the key is already in the cache, it gets moved to the head of the LRU