feat(clients): cleanup after replaceAllObjects failure

Author: millotp

scripts/cts/runCts.ts

@@ -9,6 +9,7 @@ import { startTestServer } from './testServer';
 import { printBenchmarkReport } from './testServer/benchmark.js';
 import { assertChunkWrapperValid } from './testServer/chunkWrapper.js';
 import { assertValidReplaceAllObjects } from './testServer/replaceAllObjects.js';
+import { assertValidReplaceAllObjectsFailed } from './testServer/replaceAllObjectsFailed.js';
 import { assertValidTimeouts } from './testServer/timeout.js';
 import { assertValidWaitForApiKey } from './testServer/waitFor.js';
 
@@ -152,6 +153,7 @@ export async function runCts(
     assertValidTimeouts(languages.length);
     assertChunkWrapperValid(languages.length - skip('dart') - skip('scala'));
     assertValidReplaceAllObjects(languages.length - skip('dart') - skip('scala'));
+    assertValidReplaceAllObjectsFailed(languages.length - skip('dart') - skip('scala'));
     assertValidWaitForApiKey(languages.length - skip('dart') - skip('scala'));
   }
   if (withBenchmarkServer) {

scripts/cts/testServer/index.ts

@@ -11,6 +11,7 @@ import { benchmarkServer } from './benchmark';
 import { chunkWrapperServer } from './chunkWrapper';
 import { gzipServer } from './gzip';
 import { replaceAllObjectsServer } from './replaceAllObjects';
+import { replaceAllObjectsServerFailed } from './replaceAllObjectsFailed';
 import { timeoutServer } from './timeout';
 import { timeoutServerBis } from './timeoutBis';
 import { waitForApiKeyServer } from './waitFor';
@@ -23,6 +24,7 @@ export async function startTestServer(suites: Record<CTSType, boolean>): Promise
       gzipServer(),
       timeoutServerBis(),
       replaceAllObjectsServer(),
+      replaceAllObjectsServerFailed(),
       chunkWrapperServer(),
       waitForApiKeyServer(),
       apiKeyServer(),

scripts/cts/testServer/replaceAllObjectsFailed.ts

@@ -0,0 +1,58 @@
+import type { Server } from 'http';
+
+import { expect } from 'chai';
+import type { Express } from 'express';
+import express from 'express';
+
+import { setupServer } from '.';
+
+const raoState: Record<
+  string,
+  {
+    tmpIndexName: string;
+    successful: boolean;
+  }
+> = {};
+
+export function assertValidReplaceAllObjectsFailed(expectedCount: number): void {
+  const count = Object.values(raoState).filter((s) => s.successful).length;
+  if (count !== expectedCount) {
+    throw new Error(`Expected ${expectedCount} call to replaceAllObjectsFailed, got ${count} instead.`);
+  }
+}
+
+function addRoutes(app: Express): void {
+  app.use(express.urlencoded({ extended: true }));
+  app.use(
+    express.json({
+      type: ['application/json', 'text/plain'], // the js client sends the body as text/plain
+    }),
+  );
+
+  app.post('/1/indexes/:indexName/operation', (req, res) => {
+    const lang = req.params.indexName.match(/^cts_e2e_replace_all_objects_too_big_(.*)$/)?.[1] as string;
+    raoState[lang] = {
+      tmpIndexName: req.body.destination,
+      successful: false,
+    };
+
+    res.json({ taskID: 123, updatedAt: '2021-01-01T00:00:00.000Z' });
+  });
+
+  app.post('/1/indexes/:indexName/batch', (_req, res) => {
+    res.status(400).json({ message: 'Record is too big', status: 400 });
+  });
+
+  app.delete('/1/indexes/:indexName', (req, res) => {
+    const lang = req.params.indexName.match(/^cts_e2e_replace_all_objects_too_big_(.*)_tmp/)?.[1] as string;
+    expect(raoState[lang].tmpIndexName).to.equal(req.params.indexName);
+    raoState[lang].successful = true;
+
+    res.json({ taskID: 456, deletedAt: '2021-01-01T00:00:00.000Z' });
+  });
+}
+
+export function replaceAllObjectsServerFailed(): Promise<Server> {
+  // this server is used to simulate the responses for the replaceAllObjects method, with cleanup
+  return setupServer('replaceAllObjectsFailed', 6684, addRoutes);
+}

templates/Bug_report.yml

@@ -27,7 +27,7 @@ body:
     id: client
     attributes:
       label: Client
-      description: Which API are you targetting?
+      description: Which API are you targeting?
       options:
         - All
         - AB testing

templates/go/search_helpers.mustache

@@ -653,35 +653,47 @@ func (c *APIClient) ReplaceAllObjects(indexName string, objects []map[string]any
 		return nil, err
 	}
 
-  opts = append(opts, WithWaitForTasks(true))
+	opts = append(opts, WithWaitForTasks(true))
 
 	batchResp, err := c.ChunkedBatch(tmpIndexName, objects, ACTION_ADD_OBJECT, opts...)
 	if err != nil {
+		_, _ = c.DeleteIndex(c.NewApiDeleteIndexRequest(tmpIndexName))
+
 		return nil, err
 	}
 
 	_, err = c.WaitForTask(tmpIndexName, copyResp.TaskID, toIterableOptions(opts)...)
 	if err != nil {
+		_, _ = c.DeleteIndex(c.NewApiDeleteIndexRequest(tmpIndexName))
+
 		return nil, err
 	}
 
 	copyResp, err = c.OperationIndex(c.NewApiOperationIndexRequest(indexName, NewOperationIndexParams(OPERATION_TYPE_COPY, tmpIndexName, WithOperationIndexParamsScope([]ScopeType{SCOPE_TYPE_SETTINGS, SCOPE_TYPE_RULES, SCOPE_TYPE_SYNONYMS}))), toRequestOptions(opts)...)
 	if err != nil {
+		_, _ = c.DeleteIndex(c.NewApiDeleteIndexRequest(tmpIndexName))
+
 		return nil, err
 	}
 
 	_, err = c.WaitForTask(tmpIndexName, copyResp.TaskID, toIterableOptions(opts)...)
 	if err != nil {
+		_, _ = c.DeleteIndex(c.NewApiDeleteIndexRequest(tmpIndexName))
+
 		return nil, err
 	}
 
 	moveResp, err := c.OperationIndex(c.NewApiOperationIndexRequest(tmpIndexName, NewOperationIndexParams(OPERATION_TYPE_MOVE, indexName)), toRequestOptions(opts)...)
 	if err != nil {
+		_, _ = c.DeleteIndex(c.NewApiDeleteIndexRequest(tmpIndexName))
+
 		return nil, err
 	}
 
 	_, err = c.WaitForTask(tmpIndexName, moveResp.TaskID, toIterableOptions(opts)...)
 	if err != nil {
+		_, _ = c.DeleteIndex(c.NewApiDeleteIndexRequest(tmpIndexName))
+
 		return nil, err
 	}
 

tests/CTS/client/search/replaceAllObjects.json

@@ -111,5 +111,54 @@
         }
       }
     ]
+  },
+  {
+    "testName": "replaceAllObjects should cleanup on failure",
+    "autoCreateClient": false,
+    "steps": [
+      {
+        "type": "createClient",
+        "parameters": {
+          "appId": "test-app-id",
+          "apiKey": "test-api-key",
+          "customHosts": [
+            {
+              "host": "${{localhost}}",
+              "port": 6684
+            }
+          ]
+        }
+      },
+      {
+        "type": "method",
+        "method": "replaceAllObjects",
+        "parameters": {
+          "indexName": "cts_e2e_replace_all_objects_too_big_${{language}}",
+          "objects": [
+            {
+              "objectID": "fine",
+              "body": "small obj"
+            },
+            {
+              "objectID": "toolarge",
+              "body": "something bigger than 10KB"
+            }
+          ]
+        },
+        "expected": {
+          "error": {
+            "csharp": "{\\\"message\\\":\\\"Record is too big\\\",\\\"status\\\":400}",
+            "go": "API error [400] {\\\"message\\\":\\\"Record is too big\\\",\\\"status\\\":400}",
+            "java": "Status Code: 400 - {\\\"message\\\":\\\"Record is too big\\\",\\\"status\\\":400}",
+            "javascript": "Record is too big",
+            "kotlin": "Client request(POST http://${{localhost}}:6684/1/indexes/cts_e2e_replace_all_objects_too_big_${{language}}/batch) invalid: 400 Bad Request. Text: \\\"{\\\"message\\\":\\\"Record is too big\\\",\\\"status\\\":400}\\\"",
+            "php": "Record is too big",
+            "python": "Record is too big",
+            "ruby": "Record is too big",
+            "swift": "HTTP error: Status code: 400 Message: Record is too big"
+          }
+        }
+      }
+    ]
   }
 ]