[JsonCodec, ParserCache] Improve debugging of serializability failures

Bug: T365036
Change-Id: I6c4c2a6a48d3bca4ade76a05bbd81cb4968872a3

1 files changed, 56 insertions, 22 deletions

diff --git a/includes/json/JsonCodec.php b/includes/json/JsonCodec.php
index 4005dcdca9c0..d23693add1c7 100644
--- a/includes/json/JsonCodec.php
+++ b/includes/json/JsonCodec.php
@@ -108,6 +108,11 @@ class JsonCodec implements JsonUnserializer, JsonSerializer {
 	private function serializeOne( &$value ) {
 		if ( $value instanceof JsonSerializable ) {
 			$value = $value->jsonSerialize();
+			if ( !is_array( $value ) ) {
+				// Although JsonSerializable doesn't /require/ the result to be
+				// an array, JsonCodec and JsonUnserializableTrait do.
+				throw new JsonException( "jsonSerialize didn't return array" );
+			}
 			$value[JsonConstants::COMPLEX_ANNOTATION] = true;
 			// The returned array may still have instance of JsonSerializable,
 			// stdClass, or array, so fall through to recursively handle these.
@@ -134,9 +139,10 @@ class JsonCodec implements JsonUnserializer, JsonSerializer {
 				$value[JsonConstants::COMPLEX_ANNOTATION] = true;
 			}
 		} elseif ( !is_scalar( $value ) && $value !== null ) {
-				throw new JsonException(
-					'Unable to serialize JSON.'
-				);
+			$details = is_object( $value ) ? get_class( $value ) : gettype( $value );
+			throw new JsonException(
+				'Unable to serialize JSON: ' . $details
+			);
 		}
 		return $value;
 	}
@@ -146,7 +152,7 @@ class JsonCodec implements JsonUnserializer, JsonSerializer {
 		// to json.
 		// TODO: make detectNonSerializableData not choke on cyclic structures.
 		$unserializablePath = $this->detectNonSerializableDataInternal(
-			$value, false, '$'
+			$value, false, '$', false
 		);
 		if ( $unserializablePath ) {
 			throw new JsonException(
@@ -159,8 +165,16 @@ class JsonCodec implements JsonUnserializer, JsonSerializer {
 		// Format as JSON
 		$json = FormatJson::encode( $value, false, FormatJson::ALL_OK );
 		if ( !$json ) {
+			try {
+				// Try to collect more information on the failure.
+				$details = $this->detectNonSerializableData( $value );
+			} catch ( \Throwable $t ) {
+				$details = $t->getMessage();
+			}
 			throw new JsonException(
-				'Failed to encode JSON. Error ' . json_last_error_msg()
+				'Failed to encode JSON. ' .
+				'Error: ' . json_last_error_msg() . '. ' .
+				'Details: ' . $details
 			);
 		}
 
@@ -205,12 +219,15 @@ class JsonCodec implements JsonUnserializer, JsonSerializer {
 	 * @param mixed $value
 	 * @param bool $expectUnserialize
 	 * @param string $accumulatedPath
+	 * @param bool $exhaustive Whether to (slowly) completely traverse the
+	 *  $value in order to find the precise location of a problem
 	 * @return string|null JSON path to first encountered non-serializable property or null.
 	 */
 	private function detectNonSerializableDataInternal(
 		$value,
 		bool $expectUnserialize,
-		string $accumulatedPath
+		string $accumulatedPath,
+		bool $exhaustive = false
 	): ?string {
 		if (
 			$this->canMakeNewFromValue( $value ) ||
@@ -219,30 +236,47 @@ class JsonCodec implements JsonUnserializer, JsonSerializer {
 			// Contains a conflicting use of JsonConstants::TYPE_ANNOTATION or
 			// JsonConstants::COMPLEX_ANNOTATION; in the future we might use
 			// an alternative encoding for these objects to allow them.
-			return $accumulatedPath;
+			return $accumulatedPath . ': conflicting use of protected property';
+		}
+		if ( is_object( $value ) ) {
+			if ( get_class( $value ) === stdClass::class ) {
+				$value = (array)$value;
+			} elseif (
+				$expectUnserialize ?
+				$value instanceof JsonUnserializable :
+				$value instanceof JsonSerializable
+			) {
+				if ( $exhaustive ) {
+					// Call the appropriate serialization method and recurse to
+					// ensure contents are also serializable.
+					$value = $value->jsonSerialize();
+					if ( !is_array( $value ) ) {
+						return $accumulatedPath . ": jsonSerialize didn't return array";
+					}
+				} else {
+					// Assume that serializable objects contain 100%
+					// serializable contents in their representation.
+					return null;
+				}
+			} else {
+				// Instances of classes other the \stdClass or JsonSerializable can not be serialized to JSON.
+				return $accumulatedPath . ': ' . get_class( $value );
+			}
 		}
-		if ( is_array( $value ) || (
-			is_object( $value ) && get_class( $value ) === stdClass::class
-		) ) {
-			foreach ( $value as $key => &$propValue ) {
+		if ( is_array( $value ) ) {
+			foreach ( $value as $key => $propValue ) {
 				$propValueNonSerializablePath = $this->detectNonSerializableDataInternal(
 					$propValue,
 					$expectUnserialize,
-					$accumulatedPath . '.' . $key
+					$accumulatedPath . '.' . $key,
+					$exhaustive
 				);
-				if ( $propValueNonSerializablePath ) {
+				if ( $propValueNonSerializablePath !== null ) {
 					return $propValueNonSerializablePath;
 				}
 			}
-		} elseif (
-			( $expectUnserialize && $value instanceof JsonUnserializable )
-			// Trust that JsonSerializable will correctly serialize.
-			|| ( !$expectUnserialize && $value instanceof JsonSerializable )
-		) {
-			return null;
-			// Instances of classes other the \stdClass or JsonSerializable can not be serialized to JSON.
 		} elseif ( !is_scalar( $value ) && $value !== null ) {
-			return $accumulatedPath;
+			return $accumulatedPath . ': nonscalar ' . gettype( $value );
 		}
 		return null;
 	}
@@ -258,6 +292,6 @@ class JsonCodec implements JsonUnserializer, JsonSerializer {
 	 * @since 1.36
 	 */
 	public function detectNonSerializableData( $value, bool $expectUnserialize = false ): ?string {
-		return $this->detectNonSerializableDataInternal( $value, $expectUnserialize, '$' );
+		return $this->detectNonSerializableDataInternal( $value, $expectUnserialize, '$', true );
 	}
 }