From 870ffb264d5a5bdf4c2c70dd38f2a0e10a60f022 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Wed, 15 Dec 2021 16:11:34 -0800
Subject: [PATCH] Civi::pipe - Flip default for $apiError from `array` to
 `exception`

My initial use-case will likely benefit from `array`; however, for bespoke
code that works with a pipe, the `exception` is probably easier.
---
 Civi/Pipe/PublicMethods.php                   |  2 +-
 .../phpunit/Civi/Pipe/JsonRpcSessionTest.php  | 30 +++++++++----------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/Civi/Pipe/PublicMethods.php b/Civi/Pipe/PublicMethods.php
index bf687860fd..9dafae58cc 100644
--- a/Civi/Pipe/PublicMethods.php
+++ b/Civi/Pipe/PublicMethods.php
@@ -25,7 +25,7 @@ class PublicMethods {
    *   - 'array': Traditional array format from civicrm_api(). Maximizes consistency of error data.
    *   - 'exception': Converted to an exception. Somewhat lossy. Improves out-of-box DX on stricter JSON-RPC clients.
    */
-  protected $apiError = 'array';
+  protected $apiError = 'exception';
 
   /**
    * Should API calls use permission checks?
diff --git a/tests/phpunit/Civi/Pipe/JsonRpcSessionTest.php b/tests/phpunit/Civi/Pipe/JsonRpcSessionTest.php
index 76beb5532c..2240e9090c 100644
--- a/tests/phpunit/Civi/Pipe/JsonRpcSessionTest.php
+++ b/tests/phpunit/Civi/Pipe/JsonRpcSessionTest.php
@@ -92,7 +92,7 @@ class JsonRpcSessionTest extends \CiviUnitTestCase {
 
   public function testControl() {
     $this->assertRequestResponse([
-      '{"jsonrpc":"2.0","id":"c","method":"options"}' => '{"jsonrpc":"2.0","result":{"apiCheckPermissions":true,"apiError":"array","bufferSize":524288,"responsePrefix":null},"id":"c"}',
+      '{"jsonrpc":"2.0","id":"c","method":"options"}' => '{"jsonrpc":"2.0","result":{"apiCheckPermissions":true,"apiError":"exception","bufferSize":524288,"responsePrefix":null},"id":"c"}',
       '{"jsonrpc":"2.0","id":"c","method":"options","params":{"responsePrefix":"ZZ"}}' => 'ZZ{"jsonrpc":"2.0","result":{"responsePrefix":"ZZ"},"id":"c"}',
       '{"jsonrpc":"2.0","id":"c","method": "echo","params":[123]}' => 'ZZ{"jsonrpc":"2.0","result":[123],"id":"c"}',
     ]);
@@ -111,10 +111,10 @@ class JsonRpcSessionTest extends \CiviUnitTestCase {
 
   public function testApi3ErrorModes() {
     $responses = $this->runLines([
-      // First call: Use default/traditional API error mode
+      // First call: By default, use JSON-RPC errors.
       '{"jsonrpc":"2.0","id":"bad1","method":"api3","params":["System","zznnzznnzz"]}',
-      // Second call: Bind API errors to JSON-RPC errors.
-      '{"jsonrpc":"2.0","id":"o","method":"options","params":{"apiError":"exception"}}',
+      // Second call: Use traditional API error-arrays
+      '{"jsonrpc":"2.0","id":"o","method":"options","params":{"apiError":"array"}}',
       '{"jsonrpc":"2.0","id":"bad2","method":"api3","params":["System","zznnzznnzz"]}',
     ]);
 
@@ -123,18 +123,18 @@ class JsonRpcSessionTest extends \CiviUnitTestCase {
     $decode = json_decode($responses[1], TRUE);
     $this->assertEquals('2.0', $decode['jsonrpc']);
     $this->assertEquals('bad1', $decode['id']);
-    $this->assertEquals(1, $decode['result']['is_error']);
-    $this->assertRegexp(';API.*System.*zznnzznnzz.*not exist;', $decode['result']['error_message']);
+    $this->assertRegexp(';API.*System.*zznnzznnzz.*not exist;', $decode['error']['message']);
 
     $decode = json_decode($responses[2], TRUE);
     $this->assertEquals('2.0', $decode['jsonrpc']);
     $this->assertEquals('o', $decode['id']);
-    $this->assertEquals('exception', $decode['result']['apiError']);
+    $this->assertEquals('array', $decode['result']['apiError']);
 
     $decode = json_decode($responses[3], TRUE);
     $this->assertEquals('2.0', $decode['jsonrpc']);
     $this->assertEquals('bad2', $decode['id']);
-    $this->assertRegexp(';API.*System.*zznnzznnzz.*not exist;', $decode['error']['message']);
+    $this->assertEquals(1, $decode['result']['is_error']);
+    $this->assertRegexp(';API.*System.*zznnzznnzz.*not exist;', $decode['result']['error_message']);
   }
 
   public function testApi4() {
@@ -200,10 +200,10 @@ class JsonRpcSessionTest extends \CiviUnitTestCase {
 
   public function testApi4ErrorModes() {
     $responses = $this->runLines([
-      // First call: Use default/traditional API error mode
+      // First call: By default, use JSON-RPC errors.
       '{"jsonrpc":"2.0","id":"bad1","method":"api4","params":["System","zznnzznnzz"]}',
-      // Second call: Bind API errors to JSON-RPC errors.
-      '{"jsonrpc":"2.0","id":"o","method":"options","params":{"apiError":"exception"}}',
+      // Second call: Use traditional API error-arrays
+      '{"jsonrpc":"2.0","id":"o","method":"options","params":{"apiError":"array"}}',
       '{"jsonrpc":"2.0","id":"bad2","method":"api4","params":["System","zznnzznnzz"]}',
     ]);
 
@@ -212,18 +212,18 @@ class JsonRpcSessionTest extends \CiviUnitTestCase {
     $decode = json_decode($responses[1], TRUE);
     $this->assertEquals('2.0', $decode['jsonrpc']);
     $this->assertEquals('bad1', $decode['id']);
-    $this->assertEquals(1, $decode['result']['is_error']);
-    $this->assertRegexp(';Api.*System.*zznnzznnzz.*not exist;', $decode['result']['error_message']);
+    $this->assertRegexp(';Api.*System.*zznnzznnzz.*not exist;', $decode['error']['message']);
 
     $decode = json_decode($responses[2], TRUE);
     $this->assertEquals('2.0', $decode['jsonrpc']);
     $this->assertEquals('o', $decode['id']);
-    $this->assertEquals('exception', $decode['result']['apiError']);
+    $this->assertEquals('array', $decode['result']['apiError']);
 
     $decode = json_decode($responses[3], TRUE);
     $this->assertEquals('2.0', $decode['jsonrpc']);
     $this->assertEquals('bad2', $decode['id']);
-    $this->assertRegexp(';Api.*System.*zznnzznnzz.*not exist;', $decode['error']['message']);
+    $this->assertEquals(1, $decode['result']['is_error']);
+    $this->assertRegexp(';Api.*System.*zznnzznnzz.*not exist;', $decode['result']['error_message']);
   }
 
   /**
-- 
2.25.1