Draft: Add Err() function to Response for detailed errors (opensearch…

…-project#246) * opensearchapi: Add Error type Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> * opensearchapi: Add Err() function to response Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> * opensearchapi: Remove unneded IsError check Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> * opensearchapi: Add tests for reponse.Err() function Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> * opensearchutil: Remove unneded IsError() check Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> * USER_GUIDE: Remove IsError check, improve example, add error.As example Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> * opensearchapi: Check reponse body for errors in all api types Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> * Update CHANGELOG.md Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> * opensearchapi: Adjust benchmarks to not brake on wanted Response Errors Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> * Add UPGRADING.md Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> --------- Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com> Co-authored-by: Daniel (dB.) Doubrovkine <dblock@dblock.org>
VachaShah · Mar 22, 2023 · 8d3cf4d · 8d3cf4d
1 parent 5c62768
commit 8d3cf4d
Show file tree

Hide file tree

Showing 157 changed files with 374 additions and 225 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Github workflow for changelog verification ([#172](https://github.com/opensearch-project/opensearch-go/pull/172))
 - Add Go Documentation link for the client ([#182](https://github.com/opensearch-project/opensearch-go/pull/182))
 - Support for Amazon OpenSearch Serverless ([#216](https://github.com/opensearch-project/opensearch-go/pull/216), [#259](https://github.com/opensearch-project/opensearch-go/pull/259))
+- Add Err() function to Response for detailed errors ([#246](https://github.com/opensearch-project/opensearch-go/pull/246))
 
 ### Dependencies
 
@@ -32,6 +33,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ### Changed
 
 - Workflow improvements ([#242](https://github.com/opensearch-project/opensearch-go/pull/242))
+- Opensearchapi check the response for errors ([#246](https://github.com/opensearch-project/opensearch-go/pull/246))
 
 ### Deprecated
 

diff --git a/UPGRADING.md b/UPGRADING.md
@@ -0,0 +1,33 @@
+- [Upgrading Opensearch GO Client](#upgrading-opensearch-go-client)
+    - [Upgraading to >= 3.0.0](#upgrading-to->=-3.0.0)
+        - [opensearchapi](#opensearchapi-error-handling)
+
+# Upgrading Opensearch GO Client
+
+## Upgrading to >= 3.0.0
+
+### opensearchapi error handling
+With opensearch-go >= 3.0.0 opensearchapi responses are now checked for errors.
+Prior versions only returned an error if the request failed to execute. For example if the client can't reach the server or the TLS handshake failed.
+With opensearch-go >= 3.0.0 each opensearchapi requests will return an error if the response http status code is > 299.
+The error can be parsed into the new opensearchapi.Error type by using `errors.As` to match for exceptions and get a more detailed view. See the example below.
+
+```go
+	createIndex := opensearchapi.IndicesCreateRequest{
+		Index: IndexName,
+		Body:  mapping,
+	}
+	ctx := context.Background()
+	var opensearchError *opensearchapi.Error
+	createIndexResponse, err := createIndex.Do(ctx, client)
+	// Load err into opensearchapi.Error to access the fields and tolerate if the index already exists
+	if err != nil {
+		if errors.As(err, &opensearchError) {
+			if opensearchError.Err.Type != "resource_already_exists_exception" {
+				return err
+			}
+		} else {
+			return err
+		}
+	}
+```
diff --git a/USER_GUIDE.md b/USER_GUIDE.md
@@ -13,19 +13,28 @@ search for the document, delete the document and finally delete the index.
 package main
 
 import (
-	"os"
 	"context"
 	"crypto/tls"
+	"errors"
 	"fmt"
-	opensearch "github.com/opensearch-project/opensearch-go/v2"
-	opensearchapi "github.com/opensearch-project/opensearch-go/v2/opensearchapi"
 	"net/http"
+	"os"
 	"strings"
+
+	opensearch "github.com/opensearch-project/opensearch-go/v2"
+	opensearchapi "github.com/opensearch-project/opensearch-go/v2/opensearchapi"
 )
 
 const IndexName = "go-test-index1"
 
 func main() {
+	if err := example(); err != nil {
+		fmt.Println(fmt.Sprintf("Error: %s", err))
+		os.Exit(1)
+	}
+}
+
+func example() error {
 
 	// Initialize the client with SSL/TLS enabled.
 	client, err := opensearch.NewClient(opensearch.Config{
@@ -37,8 +46,7 @@ func main() {
 		Password:  "admin",
 	})
 	if err != nil {
-		fmt.Println("cannot initialize", err)
-		os.Exit(1)
+		return err
 	}
 
 	// Print OpenSearch version information on console.
@@ -58,10 +66,18 @@ func main() {
 		Index: IndexName,
 		Body:  mapping,
 	}
-	createIndexResponse, err := createIndex.Do(context.Background(), client)
+	ctx := context.Background()
+	var opensearchError *opensearchapi.Error
+	createIndexResponse, err := createIndex.Do(ctx, client)
+	// Load err into opensearchapi.Error to access the fields and tolerate if the index already exists
 	if err != nil {
-		fmt.Println("failed to create index ", err)
-		os.Exit(1)
+		if errors.As(err, &opensearchError) {
+			if opensearchError.Err.Type != "resource_already_exists_exception" {
+				return err
+			}
+		} else {
+			return err
+		}
 	}
 	fmt.Println(createIndexResponse)
 
@@ -78,10 +94,9 @@ func main() {
 		DocumentID: docId,
 		Body:       document,
 	}
-	insertResponse, err := req.Do(context.Background(), client)
+	insertResponse, err := req.Do(ctx, client)
 	if err != nil {
-		fmt.Println("failed to insert document ", err)
-		os.Exit(1)
+		return err
 	}
 	fmt.Println(insertResponse)
 
@@ -100,23 +115,21 @@ func main() {
 		Body: content,
 	}
 
-	searchResponse, err := search.Do(context.Background(), client)
+	searchResponse, err := search.Do(ctx, client)
 	if err != nil {
-		fmt.Println("failed to search document ", err)
-		os.Exit(1)
+		return err
 	}
 	fmt.Println(searchResponse)
 
 	// Delete the document.
-	delete := opensearchapi.DeleteRequest{
+	deleteReq := opensearchapi.DeleteRequest{
 		Index:      IndexName,
 		DocumentID: docId,
 	}
 
-	deleteResponse, err := delete.Do(context.Background(), client)
+	deleteResponse, err := deleteReq.Do(ctx, client)
 	if err != nil {
-		fmt.Println("failed to delete document ", err)
-		os.Exit(1)
+		return err
 	}
 	fmt.Println("deleting document")
 	fmt.Println(deleteResponse)
@@ -126,14 +139,26 @@ func main() {
 		Index: []string{IndexName},
 	}
 
-	deleteIndexResponse, err := deleteIndex.Do(context.Background(), client)
+	deleteIndexResponse, err := deleteIndex.Do(ctx, client)
 	if err != nil {
-		fmt.Println("failed to delete index ", err)
-		os.Exit(1)
+		return err
 	}
 	fmt.Println("deleting index", deleteIndexResponse)
-}
 
+	// Try to delete the index again which failes as it does not exist
+	// Load err into opensearchapi.Error to access the fields and tolerate if the index is missing
+	_, err = deleteIndex.Do(ctx, client)
+	if err != nil {
+		if errors.As(err, &opensearchError) {
+			if opensearchError.Err.Type != "index_not_found_exception" {
+				return err
+			}
+		} else {
+			return err
+		}
+	}
+	return nil
+}
 ```
 
 ## Amazon OpenSearch Service
@@ -207,17 +232,6 @@ func main() {
 	}
 	defer resp.Body.Close()
 
-	if resp.IsError() {
-		log.Printf("ping response status: %q", resp.Status())
-
-		respBody, err := io.ReadAll(resp.Body)
-		if err != nil {
-			log.Fatalf("failed to read response body body: %v", err)
-		}
-
-		log.Fatalf("ping resp body: %s", respBody)
-	}
-
 	log.Println("PING OK")
 }
 ```
@@ -287,7 +301,7 @@ func main() {
 		Index: indexName,
 		Body:  mapping,
 	}
-	createIndexResponse, err := createIndex.Do(context.Background(), client)
+	createIndexResponse, err := createIndex.Do(ctx, client)
 	if err != nil {
 		log.Fatalf("failed to create index: %v", err)
 	}
@@ -298,7 +312,7 @@ func main() {
 		Index: []string{indexName},
 	}
 
-	deleteIndexResponse, err := deleteIndex.Do(context.Background(), client)
+	deleteIndexResponse, err := deleteIndex.Do(ctx, client)
 	if err != nil {
 		log.Fatalf("failed to delete index: %v", err)
 	}

diff --git a/opensearchapi/api.bulk.go b/opensearchapi/api.bulk.go
@@ -196,7 +196,7 @@ func (r BulkRequest) Do(ctx context.Context, transport Transport) (*Response, er
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.aliases.go b/opensearchapi/api.cat.aliases.go
@@ -180,7 +180,7 @@ func (r CatAliasesRequest) Do(ctx context.Context, transport Transport) (*Respon
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.allocation.go b/opensearchapi/api.cat.allocation.go
@@ -192,7 +192,7 @@ func (r CatAllocationRequest) Do(ctx context.Context, transport Transport) (*Res
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.cluster_manager.go b/opensearchapi/api.cat.cluster_manager.go
@@ -156,7 +156,7 @@ func (r CatClusterManagerRequest) Do(ctx context.Context, transport Transport) (
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.count.go b/opensearchapi/api.cat.count.go
@@ -171,7 +171,7 @@ func (r CatCountRequest) Do(ctx context.Context, transport Transport) (*Response
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.fielddata.go b/opensearchapi/api.cat.fielddata.go
@@ -180,7 +180,7 @@ func (r CatFielddataRequest) Do(ctx context.Context, transport Transport) (*Resp
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.health.go b/opensearchapi/api.cat.health.go
@@ -172,7 +172,7 @@ func (r CatHealthRequest) Do(ctx context.Context, transport Transport) (*Respons
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.help.go b/opensearchapi/api.cat.help.go
@@ -146,7 +146,7 @@ func (r CatHelpRequest) Do(ctx context.Context, transport Transport) (*Response,
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.indices.go b/opensearchapi/api.cat.indices.go
@@ -216,7 +216,7 @@ func (r CatIndicesRequest) Do(ctx context.Context, transport Transport) (*Respon
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.master.go b/opensearchapi/api.cat.master.go
@@ -175,7 +175,7 @@ func (r CatMasterRequest) Do(ctx context.Context, transport Transport) (*Respons
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.nodeattrs.go b/opensearchapi/api.cat.nodeattrs.go
@@ -177,7 +177,7 @@ func (r CatNodeattrsRequest) Do(ctx context.Context, transport Transport) (*Resp
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.nodes.go b/opensearchapi/api.cat.nodes.go
@@ -197,7 +197,7 @@ func (r CatNodesRequest) Do(ctx context.Context, transport Transport) (*Response
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.pending_tasks.go b/opensearchapi/api.cat.pending_tasks.go
@@ -182,7 +182,7 @@ func (r CatPendingTasksRequest) Do(ctx context.Context, transport Transport) (*R
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.plugins.go b/opensearchapi/api.cat.plugins.go
@@ -182,7 +182,7 @@ func (r CatPluginsRequest) Do(ctx context.Context, transport Transport) (*Respon
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.recovery.go b/opensearchapi/api.cat.recovery.go
@@ -194,7 +194,7 @@ func (r CatRecoveryRequest) Do(ctx context.Context, transport Transport) (*Respo
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.repositories.go b/opensearchapi/api.cat.repositories.go
@@ -177,7 +177,7 @@ func (r CatRepositoriesRequest) Do(ctx context.Context, transport Transport) (*R
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.segments.go b/opensearchapi/api.cat.segments.go
@@ -175,7 +175,7 @@ func (r CatSegmentsRequest) Do(ctx context.Context, transport Transport) (*Respo
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.shards.go b/opensearchapi/api.cat.shards.go
@@ -196,7 +196,7 @@ func (r CatShardsRequest) Do(ctx context.Context, transport Transport) (*Respons
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.

diff --git a/opensearchapi/api.cat.snapshots.go b/opensearchapi/api.cat.snapshots.go
@@ -191,7 +191,7 @@ func (r CatSnapshotsRequest) Do(ctx context.Context, transport Transport) (*Resp
 		Header:     res.Header,
 	}
 
-	return &response, nil
+	return &response, response.Err()
 }
 
 // WithContext sets the request context.