fix: propagate status code from platform api when routing wiki requests

.github/workflows/mediawiki.verify.yml

@@ -32,3 +32,9 @@ jobs:
       - run: curl -L -s -N "http://site2.localhost:8001/w/api.php" | grep -q "Main module"
       - run: curl -L -s -N "http://site2.localhost:8001/w/load.php" | grep -q "no modules were requested"
       - run: curl -L -s -N "http://site2.localhost:8001/w/rest.php" | grep -q "did not match any known handler"
+
+      - run: curl -L -s -N "http://notfound.localhost:8001/wiki/Main_Page" | grep -q "It may never have existed"
+
+      - run: curl -L -s -N "http://failwith500.localhost:8001/wiki/Main_Page" | grep -q "server error in the platform API"
+
+      - run: curl -L -s -N "http://broken.localhost:8001/wiki/Main_Page" | grep -q "server error in the platform API"

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 Tags have the format: `<MediaWiki core version>-<PHP Version>-<date>-<build number>`
 
+## 1.39-7.4-20240116-0
+- On failure, propagate status code from Platform API to clients (T343744)
+
 ## 1.39-7.4-20240103-0
 - Add QuestyCaptcha config option
 

composer.json

@@ -6,7 +6,7 @@
     "scripts": {
         "lint": [
             "parallel-lint --exclude ./dist --exclude ./sync/.tmp .",
-            "rc=0 && for x in `find . -not -path './sync/.tmp/*' -not -path './dist/*' -name \"*.json\" -type f`; do vendor/bin/jsonlint \"$x\" || rc=$?; done && exit $rc"
+            "rc=0 && for x in `find . -not -path './sync/.tmp/*' -not -path './dist/*' -name \"*.json\" ! -name \"WikiInfo-broken.json\" -type f`; do vendor/bin/jsonlint \"$x\" || rc=$?; done && exit $rc"
             ]
     }
 }

dist-persist/wbstack/src/Info/GlobalSet.php

@@ -2,34 +2,38 @@
 
 namespace WBStack\Info;
 
+class GlobalSetException extends \Exception {}
+
 /**
  * A class for setting a global holding a WBStackInfo object.
  * This includes lookup of the data for the global from cache or API.
  */
+
 class GlobalSet {
 
     /**
      * @param string $requestDomain A request domain, or 'maint' Example: 'addshore-alpha.wiki.opencura.com'
-     * @return bool was the global successfully set?
+     * @return void
      */
-    public static function forDomain( $requestDomain ) {
+    public static function forDomain($requestDomain) {
         // Normalize the domain by setting to lowercase
         $requestDomain = strtolower($requestDomain);
 
         // If the domain is 'maint' then set the maint settings
-        if($requestDomain === 'maint') {
+        if ($requestDomain === 'maint') {
             self::forMaint();
-            return true;
+            return;
         }
 
-        $info = self::getCachedOrFreshInfo( $requestDomain );
-
-        if($info === null) {
-            return false;
+        $info = self::getCachedOrFreshInfo($requestDomain);
+        if ($info instanceof WBStackLookupFailure) {
+            throw new GlobalSetException(
+                "Failure looking up wiki $requestDomain.",
+                $info->statusCode,
+            );
         }
 
-        self::setGlobal( $info );
-        return true;
+        self::setGlobal($info);
     }
 
     /**
@@ -60,7 +64,7 @@ private static function setGlobal( $info ) {
 
     /**
      * @param string $requestDomain
-     * @return WBStackInfo|null
+     * @return WBStackInfo|WBStackLookupFailure
      */
     private static function getCachedOrFreshInfo( $requestDomain ) {
         $info = self::getInfoFromApcCache( $requestDomain );
@@ -70,30 +74,29 @@ private static function getCachedOrFreshInfo( $requestDomain ) {
 
         // TODO create an APC lock saying this proc is going to get fresh data?
         // TODO in reality all of this needs to change...
-
         $info = self::getInfoFromApi( $requestDomain );
 
-        // Cache positive results for 10 seconds, negative for 2
-        $ttl = $info ? 10 : 2;
+        // Cache positive results for 10 seconds, failures for 2
+        $ttl = $info instanceof WBStackInfo ? 10 : 2;
         self::writeInfoToApcCache( $requestDomain, $info, $ttl );
 
         return $info;
     }
 
     /**
      * @param string $requestDomain
-     * @return WBStackInfo|null|bool false if no info is cached (should check), null for cached empty data (should not check)
+     * @return WBStackInfo|WBStackLookupFailure|false false if no info is cached (should check)
      */
     private static function getInfoFromApcCache( $requestDomain ) {
         return apcu_fetch( self::cacheKey($requestDomain) );
     }
 
     /**
      * @param string $requestDomain
-     * @param WBStackInfo|null $info
+     * @param WBStackInfo|WBStackLookupFailure $info
      * @param int $ttl
      */
-    private static function writeInfoToApcCache( $requestDomain, ?WBStackInfo $info, $ttl ) {
+    private static function writeInfoToApcCache( $requestDomain, $info, $ttl ) {
         $result = apcu_store( self::cacheKey($requestDomain), $info, $ttl );
         if(!$result) {
             // TODO log failed stores?!
@@ -106,7 +109,7 @@ private static function cacheKey( $requestDomain ) {
 
     /**
      * @param string $requestDomain
-     * @return WBStackInfo|null
+     * @return WBStackInfo|WBStackLookupFailure
      */
     private static function getInfoFromApi( $requestDomain ) {
         // START generic getting of wiki info from domain
@@ -120,16 +123,22 @@ private static function getInfoFromApi( $requestDomain ) {
         curl_setopt($client, CURLOPT_HTTPHEADER, $headers);
         curl_setopt($client, CURLOPT_RETURNTRANSFER, true);
         curl_setopt( $client, CURLOPT_USERAGENT, "WBStack - MediaWiki - WBStackInfo::getInfoFromApi" );
-
+        
         $response = curl_exec($client);
+        if ($response === false) {
+            return new WBStackLookupFailure(502);
+        }
+        $responseCode = intval(curl_getinfo($client, CURLINFO_RESPONSE_CODE));
 
-        // TODO detect non 200 response here, and pass that out to the user as an error
+        if ($responseCode > 399) {
+            return new WBStackLookupFailure($responseCode);
+        }
 
-        $info = WBStackInfo::newFromJsonString($response, $requestDomain);
-        if (!$info) {
-            return null;
+        try {
+            return WBStackInfo::newFromJsonString($response, $requestDomain);
+        } catch (\Exception $ex) {
+            return new WBStackLookupFailure(502);
         }
-        return $info;
     }
 
-}
+}

dist-persist/wbstack/src/Info/WBStackInfo.php

@@ -41,15 +41,15 @@ public static function newFromJsonString( $infoJsonString, $requestDomain ) {
 
         // Check if the string we were given was invalid and thus not parsed!
         if ( $data === null ) {
-            return null;
+            throw new \Exception('Unexpected malformed payload.');
         }
 
         // Get the inner data from the response
         $data = $data->data;
 
         // Data from the api should always be an array with at least an id...
         if (!is_object($data) || !property_exists($data, 'id')) {
-            return null;
+            throw new \Exception('Unexpected payload shape.');
         }
 
         $info = new self($data);
@@ -75,3 +75,12 @@ public function __get($name)
     }
 
 }
+
+class WBStackLookupFailure
+{
+    public $statusCode;
+
+    public function __construct($statusCode) {
+        $this->statusCode = $statusCode;
+    }
+}

dist-persist/wbstack/src/Shim/Cli.php

@@ -15,7 +15,9 @@
 }
 
 // Try and get the wiki info (from env var) or fail with a message
-if(!\WBStack\Info\GlobalSet::forDomain( getenv( 'WBS_DOMAIN' ) )) {
+try {
+    \WBStack\Info\GlobalSet::forDomain( getenv( 'WBS_DOMAIN' ) );
+} catch (Exception $ex) {
     echo 'Failed to work for WBS_DOMAIN: ' . getenv( 'WBS_DOMAIN' );
     die(1);
-}
+}

dist-persist/wbstack/src/Shim/Web.php

@@ -2,11 +2,18 @@
 
 require_once __DIR__ . '/../loadShim.php';
 
-// Try and get the wiki info, for fail with a 404 web page
-if(!\WBStack\Info\GlobalSet::forDomain( $_SERVER['SERVER_NAME'] ) ) {
-    http_response_code(404);
-    echo "You have requested the domain: " . $_SERVER['SERVER_NAME'] . ". But that wiki can not currently be loaded.\n";
-    echo "It may never have existed, or it might now be deleted.\n";
+// Try and get the wiki info or fail
+try {
+    \WBStack\Info\GlobalSet::forDomain($_SERVER['SERVER_NAME']);
+} catch (\WBStack\Info\GlobalSetException $ex) {
+    http_response_code($ex->getCode());
+    echo "You have requested the domain: " . $_SERVER['SERVER_NAME'] . ". But that wiki can not currently be loaded.".PHP_EOL;
+    if ($ex->getCode() === 404) {
+        echo "It may never have existed or it might now be deleted.".PHP_EOL;
+    } else {
+        echo "There was a server error in the platform API.".PHP_EOL;
+    }
+    echo $ex->getMessage();
     die(1);
 }
 

dist/wbstack/src/Info/GlobalSet.php

@@ -2,34 +2,38 @@
 
 namespace WBStack\Info;
 
+class GlobalSetException extends \Exception {}
+
 /**
  * A class for setting a global holding a WBStackInfo object.
  * This includes lookup of the data for the global from cache or API.
  */
+
 class GlobalSet {
 
     /**
      * @param string $requestDomain A request domain, or 'maint' Example: 'addshore-alpha.wiki.opencura.com'
-     * @return bool was the global successfully set?
+     * @return void
      */
-    public static function forDomain( $requestDomain ) {
+    public static function forDomain($requestDomain) {
         // Normalize the domain by setting to lowercase
         $requestDomain = strtolower($requestDomain);
 
         // If the domain is 'maint' then set the maint settings
-        if($requestDomain === 'maint') {
+        if ($requestDomain === 'maint') {
             self::forMaint();
-            return true;
+            return;
         }
 
-        $info = self::getCachedOrFreshInfo( $requestDomain );
-
-        if($info === null) {
-            return false;
+        $info = self::getCachedOrFreshInfo($requestDomain);
+        if ($info instanceof WBStackLookupFailure) {
+            throw new GlobalSetException(
+                "Failure looking up wiki $requestDomain.",
+                $info->statusCode,
+            );
         }
 
-        self::setGlobal( $info );
-        return true;
+        self::setGlobal($info);
     }
 
     /**
@@ -60,7 +64,7 @@ private static function setGlobal( $info ) {
 
     /**
      * @param string $requestDomain
-     * @return WBStackInfo|null
+     * @return WBStackInfo|WBStackLookupFailure
      */
     private static function getCachedOrFreshInfo( $requestDomain ) {
         $info = self::getInfoFromApcCache( $requestDomain );
@@ -70,30 +74,29 @@ private static function getCachedOrFreshInfo( $requestDomain ) {
 
         // TODO create an APC lock saying this proc is going to get fresh data?
         // TODO in reality all of this needs to change...
-
         $info = self::getInfoFromApi( $requestDomain );
 
-        // Cache positive results for 10 seconds, negative for 2
-        $ttl = $info ? 10 : 2;
+        // Cache positive results for 10 seconds, failures for 2
+        $ttl = $info instanceof WBStackInfo ? 10 : 2;
         self::writeInfoToApcCache( $requestDomain, $info, $ttl );
 
         return $info;
     }
 
     /**
      * @param string $requestDomain
-     * @return WBStackInfo|null|bool false if no info is cached (should check), null for cached empty data (should not check)
+     * @return WBStackInfo|WBStackLookupFailure|false false if no info is cached (should check)
      */
     private static function getInfoFromApcCache( $requestDomain ) {
         return apcu_fetch( self::cacheKey($requestDomain) );
     }
 
     /**
      * @param string $requestDomain
-     * @param WBStackInfo|null $info
+     * @param WBStackInfo|WBStackLookupFailure $info
      * @param int $ttl
      */
-    private static function writeInfoToApcCache( $requestDomain, ?WBStackInfo $info, $ttl ) {
+    private static function writeInfoToApcCache( $requestDomain, $info, $ttl ) {
         $result = apcu_store( self::cacheKey($requestDomain), $info, $ttl );
         if(!$result) {
             // TODO log failed stores?!
@@ -106,7 +109,7 @@ private static function cacheKey( $requestDomain ) {
 
     /**
      * @param string $requestDomain
-     * @return WBStackInfo|null
+     * @return WBStackInfo|WBStackLookupFailure
      */
     private static function getInfoFromApi( $requestDomain ) {
         // START generic getting of wiki info from domain
@@ -120,16 +123,22 @@ private static function getInfoFromApi( $requestDomain ) {
         curl_setopt($client, CURLOPT_HTTPHEADER, $headers);
         curl_setopt($client, CURLOPT_RETURNTRANSFER, true);
         curl_setopt( $client, CURLOPT_USERAGENT, "WBStack - MediaWiki - WBStackInfo::getInfoFromApi" );
-
+        
         $response = curl_exec($client);
+        if ($response === false) {
+            return new WBStackLookupFailure(502);
+        }
+        $responseCode = intval(curl_getinfo($client, CURLINFO_RESPONSE_CODE));
 
-        // TODO detect non 200 response here, and pass that out to the user as an error
+        if ($responseCode > 399) {
+            return new WBStackLookupFailure($responseCode);
+        }
 
-        $info = WBStackInfo::newFromJsonString($response, $requestDomain);
-        if (!$info) {
-            return null;
+        try {
+            return WBStackInfo::newFromJsonString($response, $requestDomain);
+        } catch (\Exception $ex) {
+            return new WBStackLookupFailure(502);
         }
-        return $info;
     }
 
-}
+}

dist/wbstack/src/Info/WBStackInfo.php

@@ -41,15 +41,15 @@ public static function newFromJsonString( $infoJsonString, $requestDomain ) {
 
         // Check if the string we were given was invalid and thus not parsed!
         if ( $data === null ) {
-            return null;
+            throw new \Exception('Unexpected malformed payload.');
         }
 
         // Get the inner data from the response
         $data = $data->data;
 
         // Data from the api should always be an array with at least an id...
         if (!is_object($data) || !property_exists($data, 'id')) {
-            return null;
+            throw new \Exception('Unexpected payload shape.');
         }
 
         $info = new self($data);
@@ -75,3 +75,12 @@ public function __get($name)
     }
 
 }
+
+class WBStackLookupFailure
+{
+    public $statusCode;
+
+    public function __construct($statusCode) {
+        $this->statusCode = $statusCode;
+    }
+}