fix: resolve query syntax errors and improve error handling in Request class

src/Appwrite/Utopia/Request.php

@@ -51,38 +51,47 @@ class Request extends UtopiaRequest
 
         if (!\is_array($methods)) {
             $id = $methods->getNamespace() . '.' . $methods->getMethodName();
+        } else {
+            $matched = null;
+            foreach ($methods as $method) {
+                /** @var Method|null $method */
+                if ($method === null) {
+                    continue;
+                }
+
+                // Find the method that matches the parameters passed
+                $methodParamNames = \array_map(fn ($param) => $param->getName(), $method->getParameters());
+                $invalidParams = \array_diff(\array_keys($parameters), $methodParamNames);
+
+                // No params defined, or all params are valid
+                if (empty($methodParamNames) || empty($invalidParams)) {
+                    $matched = $method;
+                    break;
+                }
+            }
+
+            $id = $matched !== null
+                ? $matched->getNamespace() . '.' . $matched->getMethodName()
+                : 'unknown.unknown';
+        }
+
+        try {
             foreach ($this->getFilters() as $filter) {
                 $parameters = $filter->parse($parameters, $id);
             }
-            $this->filteredParams = $parameters;
-            return $parameters;
-        }
-
-        $matched = null;
-        foreach ($methods as $method) {
-            /** @var Method|null $method */
-            if ($method === null) {
-                continue;
+        } catch (\Throwable $e) {
+            // 4xx filter throws are user-input errors that the action layer
+            // revalidates and reports. Cache the raw, pre-filter parameters
+            // so a subsequent getParams() — e.g. when the framework builds
+            // arguments for an error hook — returns without re-running
+            // filters. Otherwise the second throw gets wrapped as
+            // "Error handler had an error: ..." (HTTP 500), masking the
+            // intended 400.
+            $code = $e->getCode();
+            if (\is_int($code) && $code >= 400 && $code < 500) {
+                $this->filteredParams = parent::getParams();
             }
-
-            // Find the method that matches the parameters passed
-            $methodParamNames = \array_map(fn ($param) => $param->getName(), $method->getParameters());
-            $invalidParams = \array_diff(\array_keys($parameters), $methodParamNames);
-
-            // No params defined, or all params are valid
-            if (empty($methodParamNames) || empty($invalidParams)) {
-                $matched = $method;
-                break;
-            }
-        }
-
-        $id = $matched !== null
-            ? $matched->getNamespace() . '.' . $matched->getMethodName()
-            : 'unknown.unknown';
-
-        // Apply filters
-        foreach ($this->getFilters() as $filter) {
-            $parameters = $filter->parse($parameters, $id);
+            throw $e;
         }
 
         $this->filteredParams = $parameters;

tests/e2e/Services/Databases/DatabasesBase.php

@@ -4339,6 +4339,51 @@ trait DatabasesBase
         // $this->assertEquals('Invalid query: Cannot query search on attribute "actors" because it is an array.', $documents['body']['message']);
     }
 
+    public function testDocumentsListInvalidQuerySyntax(): void
+    {
+        $data = $this->setupDocuments();
+        $databaseId = $data['databaseId'];
+
+        // Each entry in `queries` must be a JSON-encoded object (e.g.
+        // `{"method":"limit","values":[5]}`). Anything else — including the
+        // legacy SDK shorthand `limit(5)`, a hand-rolled non-JSON string, or a
+        // JSON value that isn't an object — must be rejected by Query::parse
+        // with the same `Invalid query: Syntax error` message so callers get
+        // a consistent 400.
+        $invalidQueries = [
+            'legacy shorthand'   => 'limit(5)',
+            'plain string'       => 'not-json',
+            'json non-object'    => '"limit"',
+            'malformed json'     => '{"method":"limit","values":[5}',
+            'unquoted attribute' => '{method:"limit",values:[5]}',
+        ];
+
+        foreach ($invalidQueries as $label => $rawQuery) {
+            $documents = $this->client->call(Client::METHOD_GET, $this->getRecordUrl($databaseId, $data['moviesId']), array_merge([
+                'content-type' => 'application/json',
+                'x-appwrite-project' => $this->getProject()['$id'],
+            ], $this->getHeaders()), [
+                'queries' => [$rawQuery],
+            ]);
+
+            $this->assertEquals(400, $documents['headers']['status-code'], "Expected 400 for [$label]: $rawQuery");
+            $this->assertEquals('Invalid query: Syntax error', $documents['body']['message'], "Wrong error for [$label]: $rawQuery");
+        }
+
+        // Sanity check: the JSON form the SDK actually emits still works.
+        $documents = $this->client->call(Client::METHOD_GET, $this->getRecordUrl($databaseId, $data['moviesId']), array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+        ], $this->getHeaders()), [
+            'queries' => [
+                Query::limit(5)->toString(),
+            ],
+        ]);
+
+        $this->assertEquals(200, $documents['headers']['status-code']);
+        $this->assertLessThanOrEqual(5, count($documents['body'][$this->getRecordResource()]));
+    }
+
     public function testUpdateDocument(): void
     {
         $data = $this->setupDocuments();

tests/unit/Utopia/Request/Filters/ThrowingFilter.php

@@ -0,0 +1,24 @@
+<?php
+
+namespace Tests\Unit\Utopia\Request\Filters;
+
+use Appwrite\Utopia\Request\Filter;
+
+/**
+ * Test fixture: a filter that always throws, with a configurable code.
+ * Used to assert how Request::getParams() reacts to filter exceptions.
+ */
+class ThrowingFilter extends Filter
+{
+    public int $calls = 0;
+
+    public function __construct(private int $code, private string $reason)
+    {
+    }
+
+    public function parse(array $content, string $model): array
+    {
+        $this->calls++;
+        throw new \Exception($this->reason, $this->code);
+    }
+}

tests/unit/Utopia/RequestTest.php

@@ -5,10 +5,12 @@ namespace Tests\Unit\Utopia;
 use Appwrite\SDK\Method;
 use Appwrite\SDK\Parameter;
 use Appwrite\Utopia\Request;
+use Appwrite\Utopia\Request\Filter;
 use PHPUnit\Framework\TestCase;
 use Swoole\Http\Request as SwooleRequest;
 use Tests\Unit\Utopia\Request\Filters\First;
 use Tests\Unit\Utopia\Request\Filters\Second;
+use Tests\Unit\Utopia\Request\Filters\ThrowingFilter;
 use Utopia\Http\Route;
 
 class RequestTest extends TestCase
@@ -192,6 +194,105 @@ class RequestTest extends TestCase
         $this->assertSame('fallback', $request->getHeader('referer', 'fallback'));
     }
 
+    public function testGetParamsCachesRawParamsWhenFilterThrows4xx(): void
+    {
+        // Regression: when a request filter throws a 4xx exception during
+        // Request::getParams() (e.g. RequestV20 rejecting an unparseable
+        // queries[]), the framework's error path calls getParams() again to
+        // build error-hook arguments. Without caching, that second call
+        // re-runs the filter and re-throws, which the framework wraps as
+        // "Error handler had an error: ..." (HTTP 500), masking the intended
+        // 400. This test pins that behavior: the first call throws (so the
+        // action's argument resolution aborts), but the second call returns
+        // the raw, pre-filter params without re-invoking filters.
+        $filter = new ThrowingFilter(400, 'invalid input');
+
+        $this->setupSingleMethodRoute($filter);
+        $this->request->setQueryString(['foo' => 'bar']);
+
+        $threw = false;
+        try {
+            $this->request->getParams();
+        } catch (\Throwable $e) {
+            $threw = true;
+            $this->assertSame(400, $e->getCode());
+            $this->assertSame('invalid input', $e->getMessage());
+        }
+        $this->assertTrue($threw, 'First getParams() call must rethrow the filter exception.');
+        $this->assertSame(1, $filter->calls, 'Filter ran once on the first call.');
+
+        // Second call: framework's error hook arg resolution. Must return raw
+        // params without re-invoking the filter.
+        $params = $this->request->getParams();
+        $this->assertSame(['foo' => 'bar'], $params);
+        $this->assertSame(1, $filter->calls, 'Filter must not run again after a cached 4xx failure.');
+    }
+
+    public function testGetParamsDoesNotCacheRawParamsForServerError(): void
+    {
+        // 5xx filter throws indicate genuine server-side problems, not
+        // user-input mistakes. They must keep rethrowing on every call so
+        // the framework's normal error handling sees the failure each time
+        // — caching raw params would silently swallow real bugs.
+        $filter = new ThrowingFilter(500, 'boom');
+
+        $this->setupSingleMethodRoute($filter);
+        $this->request->setQueryString(['foo' => 'bar']);
+
+        for ($attempt = 1; $attempt <= 2; $attempt++) {
+            $threw = false;
+            try {
+                $this->request->getParams();
+            } catch (\Throwable $e) {
+                $threw = true;
+                $this->assertSame(500, $e->getCode());
+            }
+            $this->assertTrue($threw, "Call #$attempt must rethrow.");
+            $this->assertSame($attempt, $filter->calls, "Filter must run on call #$attempt.");
+        }
+    }
+
+    public function testGetParamsDoesNotCacheRawParamsForUncodedException(): void
+    {
+        // \Exception with the default code of 0 is treated as "unknown" and
+        // must propagate every call — same reasoning as 5xx.
+        $filter = new ThrowingFilter(0, 'unknown');
+
+        $this->setupSingleMethodRoute($filter);
+        $this->request->setQueryString(['foo' => 'bar']);
+
+        for ($attempt = 1; $attempt <= 2; $attempt++) {
+            $threw = false;
+            try {
+                $this->request->getParams();
+            } catch (\Throwable) {
+                $threw = true;
+            }
+            $this->assertTrue($threw, "Call #$attempt must rethrow.");
+            $this->assertSame($attempt, $filter->calls, "Filter must run on call #$attempt.");
+        }
+    }
+
+    /**
+     * Helper to attach a route with a single SDK method and one filter.
+     */
+    private function setupSingleMethodRoute(Filter $filter): void
+    {
+        $route = new Route(Request::METHOD_GET, '/single');
+        $route->label('sdk', new Method(
+            namespace: 'namespace',
+            group: 'group',
+            name: 'method',
+            description: 'description',
+            auth: [],
+            responses: [],
+        ));
+
+        $this->request->addHeader('EXAMPLE', 'VALUE');
+        $this->request->setRoute($route);
+        $this->request->addFilter($filter);
+    }
+
     /**
      * Helper to attach a route with multiple SDK methods to the request.
      */