objectache: Revert "throw on Closure" in WANObjectCache

This reverts commit 4e94fa8812d229bd93e4329647a6f45c88bacc67.

The extra serialization call here is not needed since storing
a value naturally results in serialization and thus exception
throwing already (hence the fatal error at T273242).
Performing serialization twice comes with a significant cost,
which has in past led to notable regressions.

Adding a try-catch makes sense, but this can be placed at a
higher level instead, as is already done by Ibfcbf0eceb4b7a36
which was already merged and back-ported, making this obsolete.

Other use cases:

- Continuous integration where HashBag is used, which doesn't
  need serialize and thus doesn't cover the Closure issue.
  Cost of serialize was removed there for performance reasons,
  but if we want to add it back, let's do so there in the natural
  place where it once was (or in MediumBag, which might be preferable
  so that we can use that same code path to gradually switch to JSON
  encoding and warn/throw for non-jsonic values).

- Regular getWithSet calls (rather than the async refresh from which
  the fatal is currently seen). For regular getWithSet calls we already
  get a useful stack trace that points directly to the line of code
  where the value and cache key are computed, so additional logging
  does not seem that valuable. But for consistency we could move the
  try-catch from Ibfcbf0eceb4b7a36e33a8 a little lower to cover all
  fetchOrRegenerate() calls, not just those from scheduleAsyncRefresh.

Bug: T273242
Change-Id: Ic649016dde7b58323a028ce6d4fe617ef62a1ff8

1 files changed, 0 insertions, 24 deletions

diff --git a/includes/libs/objectcache/wancache/WANObjectCache.php b/includes/libs/objectcache/wancache/WANObjectCache.php
index 8f3d163b6781..6353fdd039a2 100644
--- a/includes/libs/objectcache/wancache/WANObjectCache.php
+++ b/includes/libs/objectcache/wancache/WANObjectCache.php
@@ -760,8 +760,6 @@ class WANObjectCache implements
 			);
 		}
 
-		$this->checkValueSerializability( $key, $value );
-
 		// Wrap that value with time/TTL/version metadata
 		$wrapped = $this->wrap( $value, $logicalTTL ?: $ttl, $version, $now, $walltime );
 		$storeTTL = $ttl + $staleTTL;
@@ -1645,28 +1643,6 @@ class WANObjectCache implements
 	}
 
 	/**
-	 * Check for ability to serialize $value
-	 *
-	 * @param string $cacheKey
-	 * @param mixed $value
-	 */
-	private function checkValueSerializability( $cacheKey, $value ) {
-		// For now, we just try and see if serialize() will explode.
-		// In the future, we will want to warn when encountering any
-		// data that is not JSON-serializable, see T274189.
-		try {
-			serialize( $value );
-		} catch ( Exception $ex ) {
-			// Throw an exception that contains the cache key,
-			// to make it easier to find the code that supplied the offending value.
-			// This code may not be obvious from the stack trace, see T273242.
-			throw new InvalidArgumentException(
-				"Encountered unserializable value for $cacheKey: {$ex->getMessage()}", 0, $ex
-			);
-		}
-	}
-
-	/**
 	 * Get cache keys that should be collocated with their corresponding base keys
 	 *
 	 * @param string[] $baseKeys Cache keys made from makeKey()/makeGlobalKey()