Repository: amp-labs/connectors
Pull request #888 (2024-08-09 18:01:47)

feat: Handle empty HTTP response
# Problem
Any API call may return empty response. We are not explicit about it. And caller may forget to do nil check.

# Changes
I hid `common.JSONHTTPResponse.Body` field. It enforces access via `GetBody` which returns a flag indicating if response body was empty. This makes error handling explicit. Implementor of a deep connector must decide what empty response body signifies and how to address it.

# Result
The content of response body can be accessed 
* as *ajson.Node by calling `GetBody` (with flag check)
* or loaded into struct via `common.UnmarshalJSON` (empty body means empty struct, tests illustrate that)


# Review
Commits are grouped into related changes. Easier to review commit by commit.



Patch:
diff --git a/common/json.go b/common/json.go
index 44593a03..46148605 100644
--- a/common/json.go
+++ b/common/json.go
@@ -3,6 +3,7 @@ package common
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"mime"
 	"net/http"
@@ -18,6 +19,8 @@ type JSONHTTPClient struct {
 }
 
 // JSONHTTPResponse is a JSON response from an HTTP request.
+// Consider using Body to operate on ajson.Node or
+// Unmarshal into the struct of your choosing via UnmarshalJSON.
 type JSONHTTPResponse struct {
 	// bodyBytes is the raw response body. It's not JSON-unmarshalled.
 	// We keep it around so that we can unmarshal it into a struct later,
@@ -30,9 +33,20 @@ type JSONHTTPResponse struct {
 	// Headers are the HTTP headers of the response.
 	Headers http.Header
 
-	// Body is the JSON-unmarshalled response body. Aside from the fact
+	// body is the JSON-unmarshalled response body. Aside from the fact
 	// that it's JSON-unmarshalled, it's identical to bodyBytes.
-	Body *ajson.Node
+	// If there were no bytes this will be nil.
+	body *ajson.Node
+}
+
+// Body returns JSON node. If it is empty the flag will indicate so.
+// Empty response body is a special case and should be handled explicitly.
+func (j *JSONHTTPResponse) Body() (*ajson.Node, bool) {
+	if j.body == nil {
+		return nil, false
+	}
+
+	return j.body, true
 }
 
 // Get makes a GET request to the given URL and returns the response body as a JSON object.
@@ -104,11 +118,13 @@ func (j *JSONHTTPClient) Delete(ctx context.Context, url string, headers ...Head
 func parseJSONResponse(res *http.Response, body []byte) (*JSONHTTPResponse, error) {
 	// empty response body should not be parsed as JSON since it will cause ajson to err
 	if len(body) == 0 {
+		// Empty response. Both object and error are returned.
+		// Caller must check for error.
 		return &JSONHTTPResponse{
-			bodyBytes: body,
+			bodyBytes: make([]byte, 0),
 			Code:      res.StatusCode,
 			Headers:   res.Header,
-			Body:      nil,
+			body:      nil,
 		}, nil
 	}
 	// Ensure the response is JSON
@@ -137,7 +153,7 @@ func parseJSONResponse(res *http.Response, body []byte) (*JSONHTTPResponse, erro
 		bodyBytes: body,
 		Code:      res.StatusCode,
 		Headers:   res.Header,
-		Body:      jsonBody,
+		body:      jsonBody,
 	}, nil
 }
 
@@ -145,8 +161,13 @@ func parseJSONResponse(res *http.Response, body []byte) (*JSONHTTPResponse, erro
 func UnmarshalJSON[T any](rsp *JSONHTTPResponse) (*T, error) {
 	var data T
 
+	if len(rsp.bodyBytes) == 0 {
+		// Empty struct.
+		return &data, nil
+	}
+
 	if err := json.Unmarshal(rsp.bodyBytes, &data); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal response body into JSON: %w", err)
+		return nil, errors.Join(err, ErrFailedToUnmarshalBody)
 	}
 
 	return &data, nil
diff --git a/common/parse.go b/common/parse.go
index 4e9e7829..603d443a 100644
--- a/common/parse.go
+++ b/common/parse.go
@@ -25,16 +25,17 @@ func ParseResult(
 	marshalFunc func([]map[string]any, []string) ([]ReadResultRow, error),
 	fields []string,
 ) (*ReadResult, error) {
-	if resp == nil {
+	body, ok := resp.Body()
+	if !ok {
 		return nil, ErrEmptyJSONHTTPResponse
 	}
 
-	records, err := recordsFunc(resp.Body)
+	records, err := recordsFunc(body)
 	if err != nil {
 		return nil, err
 	}
 
-	nextPage, err := nextPageFunc(resp.Body)
+	nextPage, err := nextPageFunc(body)
 	if err != nil {
 		return nil, err
 	}
diff --git a/common/types.go b/common/types.go
index ff630a1f..472ad0c7 100644
--- a/common/types.go
+++ b/common/types.go
@@ -86,14 +86,14 @@ var (
 	// ErrEmptyJSONHTTPResponse is returned when the JSONHTTPResponse is nil.
 	ErrEmptyJSONHTTPResponse = errors.New("empty json http response")
 
-	// ErrEmptyResponse is returned when the response body is empty.
-	ErrEmptyResponse = errors.New("empty response body")
-
 	// ErrRecordDataNotJSON is returned when the record data in WriteParams is not JSON.
 	ErrRecordDataNotJSON = errors.New("record data is not JSON")
 
-	// ErrObjectNotSupported is returned when operation is not supported for this object.
+	// ErrOperationNotSupportedForObject is returned when operation is not supported for this object.
 	ErrOperationNotSupportedForObject = errors.New("operation is not supported for this object")
+
+	// ErrFailedToUnmarshalBody is returned when response body cannot be marshalled into some type.
+	ErrFailedToUnmarshalBody = errors.New("failed to unmarshal response body")
 )
 
 // ReadParams defines how we are reading data from a SaaS API.
diff --git a/examples/auth_connectors/adobe/adobe.go b/examples/auth_connectors/adobe/adobe.go
index 3edcd92b..e952340a 100644
--- a/examples/auth_connectors/adobe/adobe.go
+++ b/examples/auth_connectors/adobe/adobe.go
@@ -41,8 +41,13 @@ func adobeAuthExample(ctx context.Context) error {
 		return fmt.Errorf("unexpected status code: %d", response.Code)
 	}
 
+	body, ok := response.Body()
+	if !ok {
+		return fmt.Errorf("empty response: %w", common.ErrEmptyJSONHTTPResponse)
+	}
+
 	// The response body is already parsed (as JSON). You can access it like this:
-	nodes, err := response.Body.JSONPath("$.*.name")
+	nodes, err := body.JSONPath("$.*.name")
 	if err != nil {
 		return err
 	}
diff --git a/examples/auth_connectors/anthropic/anthropic.go b/examples/auth_connectors/anthropic/anthropic.go
index 63d1fbf7..b06584ad 100644
--- a/examples/auth_connectors/anthropic/anthropic.go
+++ b/examples/auth_connectors/anthropic/anthropic.go
@@ -50,8 +50,13 @@ func anthropicAuthExample(ctx context.Context) error {
 		return fmt.Errorf("unexpected status code: %d", response.Code)
 	}
 
+	body, ok := response.Body()
+	if !ok {
+		return fmt.Errorf("cannot get messages: %w", common.ErrEmptyJSONHTTPResponse)
+	}
+
 	// The response body is already parsed (as JSON). You can access it like this:
-	nodes, err := response.Body.JSONPath("$.model")
+	nodes, err := body.JSONPath("$.model")
 	if err != nil {
 		return err
 	}
diff --git a/examples/auth_connectors/blueshift/blueshift.go b/examples/auth_connectors/blueshift/blueshift.go
index 967dc181..6fd1a886 100644
--- a/examples/auth_connectors/blueshift/blueshift.go
+++ b/examples/auth_connectors/blueshift/blueshift.go
@@ -37,8 +37,13 @@ func blueshiftAuthExample(ctx context.Context) error {
 		return fmt.Errorf("unexpected status code: %d", response.Code)
 	}
 
+	body, ok := response.Body()
+	if !ok {
+		return fmt.Errorf("cannot get campaigns %w", common.ErrEmptyJSONHTTPResponse)
+	}
+
 	// The response body is already parsed (as JSON). You can access it like this:
-	nodes, err := response.Body.JSONPath("$.campaigns[0].name")
+	nodes, err := body.JSONPath("$.campaigns[0].name")
 	if err != nil {
 		return err
 	}
diff --git a/examples/auth_connectors/salesforce/salesforce.go b/examples/auth_connectors/salesforce/salesforce.go
index f3214934..f7e1adaa 100644
--- a/examples/auth_connectors/salesforce/salesforce.go
+++ b/examples/auth_connectors/salesforce/salesforce.go
@@ -53,8 +53,13 @@ func salesforceAuthExample(ctx context.Context) error {
 		return fmt.Errorf("unexpected status code: %d", response.Code)
 	}
 
+	body, ok := response.Body()
+	if !ok {
+		return fmt.Errorf("cannot get limits %w", common.ErrEmptyJSONHTTPResponse)
+	}
+
 	// The response body is already parsed (as JSON). You can access it like this:
-	nodes, err := response.Body.JSONPath("$.MassEmail.Max")
+	nodes, err := body.JSONPath("$.MassEmail.Max")
 	if err != nil {
 		return err
 	}
diff --git a/providers/apollo/metadata.go b/providers/apollo/metadata.go
index 639dea62..2139040f 100644
--- a/providers/apollo/metadata.go
+++ b/providers/apollo/metadata.go
@@ -10,8 +10,8 @@ import (
 )
 
 var (
-	perPage          string = "per_page" //nolint:gochecknoglobals
-	metadataPageSize string = "1"        //nolint:gochecknoglobals
+	perPage          = "per_page" //nolint:gochecknoglobals
+	metadataPageSize = "1"        //nolint:gochecknoglobals
 )
 
 // ListObjectMetadata creates metadata of object via reading objects using Apollo API.
@@ -46,54 +46,49 @@ func (c *Connector) ListObjectMetadata(ctx context.Context,
 		}
 
 		// Check nil response body, to avoid panic.
-		if resp == nil || resp.Body == nil {
-			metadataResult.Errors[objectName] = common.ErrEmptyResponse
+		body, ok := resp.Body()
+		if !ok {
+			metadataResult.Errors[objectName] = common.ErrEmptyJSONHTTPResponse
 
 			continue
 		}
 
-		metadata, err := parseMetadataFromResponse(resp, objectName)
+		metadata, err := parseMetadataFromResponse(body, objectName)
 		if err != nil {
 			if errors.Is(err, common.ErrMetadataLoadFailure) {
-				metadataResult.Errors[objectName] = common.ErrEmptyResponse
+				metadataResult.Errors[objectName] = common.ErrMetadataLoadFailure
+
+				continue
 			} else {
 				return nil, err
 			}
 		}
 
 		metadata.DisplayName = objectName
-		metadataResult.Result[objectName] = metadata
+		metadataResult.Result[objectName] = *metadata
 	}
 
 	return &metadataResult, nil
 }
 
-func parseMetadataFromResponse(resp *common.JSONHTTPResponse, obj string) (common.ObjectMetadata, error) {
-	bb := resp.Body.Source()
-	if bb == nil {
-		return common.ObjectMetadata{}, common.ErrEmptyResponse
-	}
-
-	metadata := common.ObjectMetadata{
-		FieldsMap: make(map[string]string),
-	}
-
-	root, err := ajson.Unmarshal(bb)
+func parseMetadataFromResponse(body *ajson.Node, objectName string) (*common.ObjectMetadata, error) {
+	arr, err := jsonquery.New(body).Array(objectName, true)
 	if err != nil {
-		return common.ObjectMetadata{}, err
+		return nil, err
 	}
 
-	resultArr, err := jsonquery.New(root).Array(obj, true)
-	if err != nil {
-		return common.ObjectMetadata{}, err
-	}
+	fieldsMap := make(map[string]string)
 
-	objectResponnse := resultArr[0].MustObject()
+	if len(arr) != 0 {
+		objectResponse := arr[0].MustObject()
 
-	// Using the result data to generate the metadata.
-	for k := range objectResponnse {
-		metadata.FieldsMap[k] = k
+		// Using the result data to generate the metadata.
+		for k := range objectResponse {
+			fieldsMap[k] = k
+		}
 	}
 
-	return metadata, nil
+	return &common.ObjectMetadata{
+		FieldsMap: fieldsMap,
+	}, nil
 }
diff --git a/providers/apollo/write.go b/providers/apollo/write.go
index 4ec7aaa9..944ddf9f 100644
--- a/providers/apollo/write.go
+++ b/providers/apollo/write.go
@@ -32,7 +32,14 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 		return nil, err
 	}
 
-	return constructWriteResult(json.Body, config.ObjectName)
+	body, ok := json.Body()
+	if !ok {
+		return &common.WriteResult{
+			Success: true,
+		}, nil
+	}
+
+	return constructWriteResult(body, config.ObjectName)
 }
 
 func constructWriteResult(body *ajson.Node, objName string) (*common.WriteResult, error) {
diff --git a/providers/atlassian/authmetadata.go b/providers/atlassian/authmetadata.go
index effdbc09..775976b7 100644
--- a/providers/atlassian/authmetadata.go
+++ b/providers/atlassian/authmetadata.go
@@ -64,11 +64,12 @@ func (c *Connector) retrieveCloudId(ctx context.Context) (string, error) {
 		return "", err
 	}
 
-	if res.Body == nil {
-		return "", common.ErrEmptyJSONHTTPResponse
+	body, ok := res.Body()
+	if !ok {
+		return "", errors.Join(ErrContainerNotFound, common.ErrEmptyJSONHTTPResponse)
 	}
 
-	arr, err := res.Body.GetArray()
+	arr, err := body.GetArray()
 	if err != nil {
 		return "", err
 	}
diff --git a/providers/atlassian/metadata.go b/providers/atlassian/metadata.go
index 41d1bf74..03222cea 100644
--- a/providers/atlassian/metadata.go
+++ b/providers/atlassian/metadata.go
@@ -22,7 +22,12 @@ func (c *Connector) ListObjectMetadata(ctx context.Context, _ []string) (*common
 		return nil, err
 	}
 
-	fields, err := c.parseFieldsJiraIssue(rsp.Body)
+	body, ok := rsp.Body()
+	if !ok {
+		return nil, errors.Join(ErrMissingMetadata, common.ErrEmptyJSONHTTPResponse)
+	}
+
+	fields, err := c.parseFieldsJiraIssue(body)
 	if err != nil {
 		return nil, errors.Join(ErrParsingMetadata, err)
 	}
diff --git a/providers/atlassian/write.go b/providers/atlassian/write.go
index c94459d0..63de3d19 100644
--- a/providers/atlassian/write.go
+++ b/providers/atlassian/write.go
@@ -36,14 +36,15 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 		return nil, err
 	}
 
-	if res == nil || res.Body == nil {
+	body, ok := res.Body()
+	if !ok {
 		return &common.WriteResult{
 			Success: true,
 		}, nil
 	}
 
 	// Write response has a reference to the resource but no payload data.
-	return constructWriteResult(res.Body)
+	return constructWriteResult(body)
 }
 
 func constructWriteResult(body *ajson.Node) (*common.WriteResult, error) {
diff --git a/providers/docusign/metadata.go b/providers/docusign/metadata.go
index e80cb55a..e5f53d09 100644
--- a/providers/docusign/metadata.go
+++ b/providers/docusign/metadata.go
@@ -22,10 +22,15 @@ func (c *Connector) GetPostAuthInfo(ctx context.Context) (*common.PostAuthInfo,
 		return nil, err
 	}
 
+	body, ok := resp.Body()
+	if !ok {
+		return nil, errors.Join(ErrNoAccounts, common.ErrEmptyJSONHTTPResponse)
+	}
+
 	var postAuthInfo common.PostAuthInfo
 	postAuthInfo.RawResponse = resp
 
-	accounts, err := resp.Body.GetKey("accounts")
+	accounts, err := body.GetKey("accounts")
 	if err != nil {
 		return nil, err
 	}
diff --git a/providers/dynamicscrm/read.go b/providers/dynamicscrm/read.go
index 66b32c6a..81bd5eb9 100644
--- a/providers/dynamicscrm/read.go
+++ b/providers/dynamicscrm/read.go
@@ -2,6 +2,7 @@ package dynamicscrm
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -75,9 +76,10 @@ func (c *Connector) performGetRequest(ctx context.Context, url *urlbuilder.URL)
 		return nil, err
 	}
 
-	if rsp.Body == nil {
-		return nil, ErrObjectNotFound
+	body, ok := rsp.Body()
+	if !ok {
+		return nil, errors.Join(ErrObjectNotFound, common.ErrEmptyJSONHTTPResponse)
 	}
 
-	return rsp.Body, nil
+	return body, nil
 }
diff --git a/providers/hubspot/metadata.go b/providers/hubspot/metadata.go
index 89cb00a2..e501ec52 100644
--- a/providers/hubspot/metadata.go
+++ b/providers/hubspot/metadata.go
@@ -93,7 +93,12 @@ func (c *Connector) describeObject(ctx context.Context, objectName string) (*com
 		return nil, fmt.Errorf("error fetching HubSpot fields: %w", err)
 	}
 
-	rawResponse, err := ajson.Marshal(rsp.Body)
+	body, ok := rsp.Body()
+	if !ok {
+		return nil, fmt.Errorf("cannot get HubSpot fields %w", common.ErrEmptyJSONHTTPResponse)
+	}
+
+	rawResponse, err := ajson.Marshal(body)
 	if err != nil {
 		return nil, fmt.Errorf("error marshalling object metadata response into byte array: %w", err)
 	}
diff --git a/providers/instantly/write.go b/providers/instantly/write.go
index 231ca1f0..3c7a5e08 100644
--- a/providers/instantly/write.go
+++ b/providers/instantly/write.go
@@ -35,9 +35,11 @@ func (c *Connector) Write(
 	var write common.WriteMethod
 	if len(config.RecordId) == 0 {
 		write = c.Client.Post
+
 		constructURLPathCreate(config, url) // nolint:wsl
 	} else {
 		write = c.Client.Patch
+
 		constructURLPathUpdate(config, url)
 	}
 
@@ -46,7 +48,8 @@ func (c *Connector) Write(
 		return nil, err
 	}
 
-	if res == nil || res.Body == nil {
+	body, ok := res.Body()
+	if !ok {
 		// it is unlikely to have no payload
 		return &common.WriteResult{
 			Success: true,
@@ -56,7 +59,7 @@ func (c *Connector) Write(
 	recordIdNodePath := recordIdPaths[config.ObjectName]
 
 	// write response was with payload
-	return constructWriteResult(res.Body, recordIdNodePath)
+	return constructWriteResult(body, recordIdNodePath)
 }
 
 var recordIdPaths = map[string]*string{ // nolint:gochecknoglobals
diff --git a/providers/intercom/write.go b/providers/intercom/write.go
index 3b5ee9f5..4d8d3e36 100644
--- a/providers/intercom/write.go
+++ b/providers/intercom/write.go
@@ -35,7 +35,8 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 		return nil, err
 	}
 
-	if res == nil || res.Body == nil {
+	body, ok := res.Body()
+	if !ok {
 		// it is unlikely to have no payload
 		return &common.WriteResult{
 			Success: true,
@@ -43,7 +44,7 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 	}
 
 	// write response was with payload
-	return constructWriteResult(res.Body)
+	return constructWriteResult(body)
 }
 
 func constructWriteResult(body *ajson.Node) (*common.WriteResult, error) {
diff --git a/providers/marketo/metadata.go b/providers/marketo/metadata.go
index 4aba7f6c..ecf54bf6 100644
--- a/providers/marketo/metadata.go
+++ b/providers/marketo/metadata.go
@@ -2,7 +2,6 @@ package marketo
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 
 	"github.com/amp-labs/connectors/common"
@@ -17,7 +16,7 @@ type responseObject struct {
 
 // ListObjectMetadata creates metadata of object via reading objects using Marketo API.
 // If it fails to fretrieve the metadata, It tried using static schema file in metadata dir.
-func (c *Connector) ListObjectMetadata(ctx context.Context, //
+func (c *Connector) ListObjectMetadata(ctx context.Context,
 	objectNames []string,
 ) (*common.ListObjectMetadataResult, error) {
 	// Ensure that objectNames is not empty
@@ -38,7 +37,13 @@ func (c *Connector) ListObjectMetadata(ctx context.Context, //
 		}
 
 		resp, err := c.Client.Get(ctx, url.String())
-		if err != nil || resp == nil || resp.Body == nil {
+		if err != nil {
+			runFallback(obj, &metadataResult)
+
+			continue
+		}
+
+		if _, ok := resp.Body(); !ok {
 			runFallback(obj, &metadataResult)
 
 			continue
@@ -46,7 +51,7 @@ func (c *Connector) ListObjectMetadata(ctx context.Context, //
 
 		metadata, err := parseMetadataFromResponse(resp)
 		if err != nil {
-			if errors.Is(err, common.ErrEmptyResponse) {
+			if errors.Is(err, common.ErrMetadataLoadFailure) {
 				runFallback(obj, &metadataResult)
 
 				continue
@@ -56,31 +61,24 @@ func (c *Connector) ListObjectMetadata(ctx context.Context, //
 		}
 
 		metadata.DisplayName = obj
-		metadataResult.Result[obj] = metadata
+		metadataResult.Result[obj] = *metadata
 	}
 
 	return &metadataResult, nil
 }
 
-func parseMetadataFromResponse(resp *common.JSONHTTPResponse) (common.ObjectMetadata, error) {
-	var response responseObject
-
-	bbytes := resp.Body.Source()
-	if bbytes == nil {
-		return common.ObjectMetadata{}, common.ErrEmptyResponse
-	}
-
-	metadata := common.ObjectMetadata{
-		FieldsMap: make(map[string]string),
-	}
-
-	err := json.Unmarshal(bbytes, &response)
+func parseMetadataFromResponse(resp *common.JSONHTTPResponse) (*common.ObjectMetadata, error) {
+	response, err := common.UnmarshalJSON[responseObject](resp)
 	if err != nil {
-		return metadata, err
+		return nil, err
 	}
 
 	if len(response.Result) == 0 {
-		return metadata, common.ErrEmptyResponse
+		return nil, common.ErrMetadataLoadFailure
+	}
+
+	metadata := &common.ObjectMetadata{
+		FieldsMap: make(map[string]string),
 	}
 
 	// Using the first result data to generate the metadata.
diff --git a/providers/outreach/metadata.go b/providers/outreach/metadata.go
index d859b4fe..0393e46e 100644
--- a/providers/outreach/metadata.go
+++ b/providers/outreach/metadata.go
@@ -2,7 +2,6 @@ package outreach
 
 import (
 	"context"
-	"encoding/json"
 
 	"github.com/amp-labs/connectors/common"
 )
@@ -45,35 +44,26 @@ func (c *Connector) ListObjectMetadata(ctx context.Context,
 			continue
 		}
 
-		// Check nil response body, to avoid panic.
-		if res == nil || res.Body == nil {
-			objMetadata.Errors[obj] = common.ErrEmptyResponse
-
-			continue
-		}
-
-		metadata, err := metadataMapper(res.Body.Source())
+		metadata, err := metadataMapper(res)
 		if err != nil {
 			return nil, err
 		}
 
 		metadata.DisplayName = obj
-		objMetadata.Result[obj] = metadata
+		objMetadata.Result[obj] = *metadata
 	}
 
 	return &objMetadata, nil
 }
 
-func metadataMapper(body []byte) (common.ObjectMetadata, error) {
-	metadata := common.ObjectMetadata{
-		FieldsMap: make(map[string]string),
+func metadataMapper(resp *common.JSONHTTPResponse) (*common.ObjectMetadata, error) {
+	response, err := common.UnmarshalJSON[Data](resp)
+	if err != nil {
+		return nil, err
 	}
 
-	var response Data
-
-	err := json.Unmarshal(body, &response)
-	if err != nil {
-		return metadata, err
+	metadata := &common.ObjectMetadata{
+		FieldsMap: make(map[string]string),
 	}
 
 	attributes := response.Data[0].Attributes
diff --git a/providers/outreach/write.go b/providers/outreach/write.go
index ada53932..7d81f0db 100644
--- a/providers/outreach/write.go
+++ b/providers/outreach/write.go
@@ -2,7 +2,6 @@ package outreach
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"strconv"
 
@@ -55,9 +54,7 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 		return nil, err
 	}
 
-	var response WriteResponse
-
-	err = json.Unmarshal(res.Body.Source(), &response)
+	response, err := common.UnmarshalJSON[WriteResponse](res)
 	if err != nil {
 		return nil, err
 	}
diff --git a/providers/pipeliner/write.go b/providers/pipeliner/write.go
index 97793724..fada8acc 100644
--- a/providers/pipeliner/write.go
+++ b/providers/pipeliner/write.go
@@ -35,15 +35,15 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 		return nil, err
 	}
 
-	if res == nil || res.Body == nil {
-		// it is unlikely to have no payload
+	body, ok := res.Body()
+	if !ok {
 		return &common.WriteResult{
 			Success: true,
 		}, nil
 	}
 
 	// write response was with payload
-	return constructWriteResult(res.Body)
+	return constructWriteResult(body)
 }
 
 func constructWriteResult(body *ajson.Node) (*common.WriteResult, error) {
diff --git a/providers/salesforce/bulk.go b/providers/salesforce/bulk.go
index 06f984ff..053e4950 100644
--- a/providers/salesforce/bulk.go
+++ b/providers/salesforce/bulk.go
@@ -314,7 +314,12 @@ func (c *Connector) bulkOperation(
 		return nil, fmt.Errorf("createJob failed: %w", err)
 	}
 
-	resObject, err := res.Body.GetObject()
+	body, ok := res.Body() // nolint:varnamelen
+	if !ok {
+		return nil, fmt.Errorf("createJob failed: %w", common.ErrEmptyJSONHTTPResponse)
+	}
+
+	resObject, err := body.GetObject()
 	if err != nil {
 		return nil, fmt.Errorf(
 			"parsing result of createJob failed: %w",
@@ -358,7 +363,12 @@ func (c *Connector) bulkOperation(
 		return nil, fmt.Errorf("completeUpload failed: %w", err)
 	}
 
-	dataObject, err := data.Body.GetObject()
+	body, ok = data.Body()
+	if !ok {
+		return nil, fmt.Errorf("completeUpload failed: %w", common.ErrEmptyJSONHTTPResponse)
+	}
+
+	dataObject, err := body.GetObject()
 	if err != nil {
 		return nil, fmt.Errorf("parsing result of completeUpload failed: %w", errors.Join(err, common.ErrParseError))
 	}
diff --git a/providers/salesforce/events.go b/providers/salesforce/events.go
index 7855bdf3..3d99a8d0 100644
--- a/providers/salesforce/events.go
+++ b/providers/salesforce/events.go
@@ -221,7 +221,12 @@ func (c *Connector) getOrganization(ctx context.Context) (map[string]*ajson.Node
 		return nil, err
 	}
 
-	return resp.Body.GetObject()
+	body, ok := resp.Body()
+	if !ok {
+		return nil, fmt.Errorf("cannot get organization %w", common.ErrEmptyJSONHTTPResponse)
+	}
+
+	return body.GetObject()
 }
 
 func (c *Connector) GetOrganizationId(ctx context.Context) (string, error) {
@@ -266,6 +271,10 @@ func (c *Connector) postToSFAPI(ctx context.Context, body any, path string, enti
 	}
 
 	res, err := common.UnmarshalJSON[SFAPIResponseBody](resp)
+	if err != nil {
+		return nil, err
+	}
+
 	if res.Warnings != nil && len(res.Warnings) > 0 {
 		slog.Warn(entity, "warnings", res.Warnings)
 	}
diff --git a/providers/salesforce/write.go b/providers/salesforce/write.go
index 1944a173..c8c0ebaa 100644
--- a/providers/salesforce/write.go
+++ b/providers/salesforce/write.go
@@ -35,24 +35,24 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 
 // parseWriteResult parses the response from writing to Salesforce API. A 2xx return type is assumed.
 func parseWriteResult(rsp *common.JSONHTTPResponse) (*common.WriteResult, error) {
-	// in case we got a 204 and empty array => unmarshal into nil ajson node
-	if rsp == nil || rsp.Body == nil {
+	body, ok := rsp.Body()
+	if !ok {
 		return &common.WriteResult{
 			Success: true,
 		}, nil
 	}
 
-	recordID, err := jsonquery.New(rsp.Body).Str("id", false)
+	recordID, err := jsonquery.New(body).Str("id", false)
 	if err != nil {
 		return nil, err
 	}
 
-	errors, err := getErrors(rsp.Body)
+	errors, err := getErrors(body)
 	if err != nil {
 		return nil, err
 	}
 
-	success, err := jsonquery.New(rsp.Body).Bool("success", false)
+	success, err := jsonquery.New(body).Bool("success", false)
 	if err != nil {
 		return nil, err
 	}
diff --git a/providers/salesloft/write.go b/providers/salesloft/write.go
index 21dff799..157ed090 100644
--- a/providers/salesloft/write.go
+++ b/providers/salesloft/write.go
@@ -36,7 +36,8 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 		return nil, err
 	}
 
-	if res == nil || res.Body == nil {
+	body, ok := res.Body()
+	if !ok {
 		// it is unlikely to have no payload
 		return &common.WriteResult{
 			Success: true,
@@ -44,7 +45,7 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 	}
 
 	// write response was with payload
-	return constructWriteResult(res.Body)
+	return constructWriteResult(body)
 }
 
 func constructWriteResult(body *ajson.Node) (*common.WriteResult, error) {
diff --git a/providers/smartlead/write.go b/providers/smartlead/write.go
index 6d618e9f..3ee5e160 100644
--- a/providers/smartlead/write.go
+++ b/providers/smartlead/write.go
@@ -42,7 +42,8 @@ func (c *Connector) Write(
 		return nil, err
 	}
 
-	if res == nil || res.Body == nil {
+	body, ok := res.Body()
+	if !ok {
 		// it is unlikely to have no payload
 		return &common.WriteResult{
 			Success: true,
@@ -52,7 +53,7 @@ func (c *Connector) Write(
 	recordIdNodePath := recordIdPaths[config.ObjectName]
 
 	// write response was with payload
-	return constructWriteResult(res.Body, recordIdNodePath)
+	return constructWriteResult(body, recordIdNodePath)
 }
 
 var recordIdPaths = map[string]string{ // nolint:gochecknoglobals
diff --git a/providers/zendesksupport/write.go b/providers/zendesksupport/write.go
index 6d3c35c1..0a655200 100644
--- a/providers/zendesksupport/write.go
+++ b/providers/zendesksupport/write.go
@@ -37,7 +37,8 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 		return nil, err
 	}
 
-	if res == nil || res.Body == nil {
+	body, ok := res.Body()
+	if !ok {
 		// it is unlikely to have no payload
 		return &common.WriteResult{
 			Success: true,
@@ -45,7 +46,7 @@ func (c *Connector) Write(ctx context.Context, config common.WriteParams) (*comm
 	}
 
 	// write response was with payload
-	return constructWriteResult(config, res.Body)
+	return constructWriteResult(config, body)
 }
 
 func constructWriteResult(config common.WriteParams, body *ajson.Node) (*common.WriteResult, error) {


Test patch:
diff --git a/common/json_test.go b/common/json_test.go
new file mode 100644
index 00000000..afe04c00
--- /dev/null
+++ b/common/json_test.go
@@ -0,0 +1,70 @@
+package common
+
+import (
+	"testing"
+
+	"github.com/amp-labs/connectors/test/utils/testutils"
+)
+
+func TestUnmarshalJSON(t *testing.T) { // nolint:funlen
+	t.Parallel()
+
+	type Story struct {
+		Title       string `json:"title"`
+		Description string `json:"description"`
+	}
+
+	tests := []struct {
+		name        string
+		input       []byte
+		expected    *Story
+		expectedErr error
+	}{
+		{
+			name:  "Nil body produces empty struct",
+			input: nil,
+			expected: &Story{
+				Title:       "",
+				Description: "",
+			},
+			expectedErr: nil,
+		},
+		{
+			name:  "Empty body produces empty struct",
+			input: []byte(""),
+			expected: &Story{
+				Title:       "",
+				Description: "",
+			},
+			expectedErr: nil,
+		},
+		{
+			name:        "Invalid JSON produces marshal error",
+			input:       []byte("2359"),
+			expected:    nil,
+			expectedErr: ErrFailedToUnmarshalBody,
+		},
+		{
+			name:  "Valid JSON values are mapped to struct fields",
+			input: []byte(`{"title": "Amazing", "description": "very long story"}`),
+			expected: &Story{
+				Title:       "Amazing",
+				Description: "very long story",
+			},
+			expectedErr: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		// nolint:varnamelen
+		tt := tt // rebind, omit loop side effects for parallel goroutine
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			output, err := UnmarshalJSON[Story](&JSONHTTPResponse{
+				bodyBytes: tt.input,
+			})
+			testutils.CheckOutputWithError(t, tt.name, tt.expected, tt.expectedErr, output, err)
+		})
+	}
+}
diff --git a/common/jsonquery/queries_test.go b/common/jsonquery/queries_test.go
index 3ae1d1e3..6219758f 100644
--- a/common/jsonquery/queries_test.go
+++ b/common/jsonquery/queries_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/amp-labs/connectors/common/handy"
+	"github.com/amp-labs/connectors/test/utils/testutils"
 	"github.com/spyzhov/ajson"
 )
 
@@ -123,7 +124,7 @@ func TestQueryInteger(t *testing.T) { // nolint:funlen
 			t.Parallel()
 
 			output, err := New(j, tt.input.zoom...).Integer(tt.input.key, tt.input.optional)
-			assertJSONManagerOutput(t, tt.name, tt.expected, tt.expectedErr, output, err)
+			testutils.CheckOutputWithError(t, tt.name, tt.expected, tt.expectedErr, output, err)
 		})
 	}
 }
@@ -218,7 +219,7 @@ func TestQueryString(t *testing.T) { // nolint:funlen
 			t.Parallel()
 
 			output, err := New(j, tt.input.zoom...).Str(tt.input.key, tt.input.optional)
-			assertJSONManagerOutput(t, tt.name, tt.expected, tt.expectedErr, output, err)
+			testutils.CheckOutputWithError(t, tt.name, tt.expected, tt.expectedErr, output, err)
 		})
 	}
 }
@@ -313,7 +314,7 @@ func TestQueryBool(t *testing.T) { // nolint:funlen
 			t.Parallel()
 
 			output, err := New(j, tt.input.zoom...).Bool(tt.input.key, tt.input.optional)
-			assertJSONManagerOutput(t, tt.name, tt.expected, tt.expectedErr, output, err)
+			testutils.CheckOutputWithError(t, tt.name, tt.expected, tt.expectedErr, output, err)
 		})
 	}
 }
@@ -519,18 +520,3 @@ func helperCreateJSON(t *testing.T, text string) *ajson.Node {
 
 	return jsonBody
 }
-
-func assertJSONManagerOutput(t *testing.T, name string, expected any, expectedErr error,
-	output any, err error,
-) {
-	t.Helper()
-
-	// check for actual error value
-	if !errors.Is(err, expectedErr) {
-		t.Fatalf("%s: expected: (%v), got: (%v)", name, expectedErr, err)
-	}
-
-	if !reflect.DeepEqual(output, expected) {
-		t.Fatalf("%s: expected: (%v), got: (%v)", name, expected, output)
-	}
-}
diff --git a/providers/atlassian/metadata_test.go b/providers/atlassian/metadata_test.go
index 5949ea52..a74d801c 100644
--- a/providers/atlassian/metadata_test.go
+++ b/providers/atlassian/metadata_test.go
@@ -36,7 +36,7 @@ func TestListObjectMetadata(t *testing.T) { // nolint:funlen,gocognit,cyclop
 				w.WriteHeader(http.StatusOK)
 				mockutils.WriteBody(w, ``)
 			})),
-			ExpectedErrs: []error{ErrParsingMetadata},
+			ExpectedErrs: []error{common.ErrEmptyJSONHTTPResponse},
 		},
 		{
 			Name:  "Server response must have at least one field",
diff --git a/providers/dynamicscrm/metadata_test.go b/providers/dynamicscrm/metadata_test.go
index a5847afc..6d3dd223 100644
--- a/providers/dynamicscrm/metadata_test.go
+++ b/providers/dynamicscrm/metadata_test.go
@@ -41,7 +41,7 @@ func TestListObjectMetadata(t *testing.T) { // nolint:funlen,gocognit,cyclop
 			Server: httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				w.Header().Set("Content-Type", "application/json")
 				w.WriteHeader(http.StatusOK)
-				_, _ = w.Write([]byte{})
+				_, _ = w.Write(responseContactsSchema)
 			})),
 			ExpectedErrs: []error{ErrObjectNotFound},
 		},
diff --git a/providers/salesforce/write_test.go b/providers/salesforce/write_test.go
index 721bb805..7fc4cad4 100644
--- a/providers/salesforce/write_test.go
+++ b/providers/salesforce/write_test.go
@@ -62,21 +62,6 @@ func TestWrite(t *testing.T) { // nolint:funlen,cyclop
 				errors.New("Unable to create/update fields: MasterRecordId"), // nolint:goerr113
 			},
 		},
-		{
-			Name:  "Write must act as a Create",
-			Input: common.WriteParams{ObjectName: "account"},
-			Server: httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				w.Header().Set("Content-Type", "application/json")
-				mockutils.RespondToMethod(w, r, "POST", func() {
-					// cannot be update path
-					mockutils.RespondToMissingQueryParameters(w, r, []string{"_HttpMethod"}, func() {
-						w.WriteHeader(http.StatusOK)
-					})
-				})
-			})),
-			Expected:     &common.WriteResult{Success: true},
-			ExpectedErrs: nil,
-		},
 		{
 			Name:  "Write must act as an Update",
 			Input: common.WriteParams{ObjectName: "account", RecordId: "003ak000004dQCUAA2"},
@@ -87,10 +72,16 @@ func TestWrite(t *testing.T) { // nolint:funlen,cyclop
 						"_HttpMethod": []string{"PATCH"},
 					}, func() {
 						w.WriteHeader(http.StatusOK)
+						_, _ = w.Write(responseCreateOK)
 					})
 				})
 			})),
-			Expected:     &common.WriteResult{Success: true},
+			Expected: &common.WriteResult{
+				Success:  true,
+				RecordId: "001ak00000OQTieAAH",
+				Errors:   []any{},
+				Data:     nil,
+			},
 			ExpectedErrs: nil,
 		},
 		{
diff --git a/test/apollo/metadata/metadata.go b/test/apollo/metadata/metadata.go
index 9b40d23b..9eb7f846 100644
--- a/test/apollo/metadata/metadata.go
+++ b/test/apollo/metadata/metadata.go
@@ -9,7 +9,6 @@ import (
 )
 
 func main() {
-
 	ctx := context.Background()
 
 	conn := apollo.GetApolloConnector(ctx, "apollo-creds.json")
diff --git a/test/apollo/read/read.go b/test/apollo/read/read.go
index af984614..f3fd4f38 100644
--- a/test/apollo/read/read.go
+++ b/test/apollo/read/read.go
@@ -41,7 +41,6 @@ func MainFn() int {
 }
 
 func testReadOpportunitiesSearch(ctx context.Context, conn *ap.Connector) error {
-
 	params := common.ReadParams{
 		ObjectName: "opportunities",
 		Fields:     []string{"id"},
@@ -65,7 +64,6 @@ func testReadOpportunitiesSearch(ctx context.Context, conn *ap.Connector) error
 }
 
 func testReadEmailAccounts(ctx context.Context, conn *ap.Connector) error {
-
 	params := common.ReadParams{
 		ObjectName: "email_accounts",
 		Fields:     []string{"user_id", "id", "email"},
@@ -90,7 +88,6 @@ func testReadEmailAccounts(ctx context.Context, conn *ap.Connector) error {
 }
 
 func testReadCustomFields(ctx context.Context, conn *ap.Connector) error {
-
 	params := common.ReadParams{
 		ObjectName: "typed_custom_fields",
 		Fields:     []string{"type", "id", "modality"},
diff --git a/test/apollo/search/search.go b/test/apollo/search/search.go
index bfbc5de2..4d16cd4a 100644
--- a/test/apollo/search/search.go
+++ b/test/apollo/search/search.go
@@ -40,7 +40,6 @@ func MainFn() int {
 }
 
 func testReadContactsSearch(ctx context.Context, conn *ap.Connector) error {
-
 	params := common.ReadParams{
 		ObjectName: "contacts",
 		Fields:     []string{"id"},
@@ -65,7 +64,6 @@ func testReadContactsSearch(ctx context.Context, conn *ap.Connector) error {
 }
 
 func testReadOpportunitiesSearch(ctx context.Context, conn *ap.Connector) error {
-
 	params := common.ReadParams{
 		ObjectName: "opportunities",
 		Fields:     []string{"id"},
@@ -89,7 +87,6 @@ func testReadOpportunitiesSearch(ctx context.Context, conn *ap.Connector) error
 }
 
 func testReadPeopleSearch(ctx context.Context, conn *ap.Connector) error {
-
 	params := common.ReadParams{
 		ObjectName: "mixed_people",
 		Fields:     []string{"id"},
diff --git a/test/utils/testutils/checks.go b/test/utils/testutils/checks.go
new file mode 100644
index 00000000..c51df843
--- /dev/null
+++ b/test/utils/testutils/checks.go
@@ -0,0 +1,23 @@
+package testutils
+
+import (
+	"errors"
+	"reflect"
+	"testing"
+)
+
+func CheckOutputWithError(t *testing.T, name string,
+	expected any, expectedErr error,
+	actual any, actualErr error,
+) {
+	t.Helper()
+
+	// check for actual error value
+	if !errors.Is(actualErr, expectedErr) {
+		t.Fatalf("%s: expected: (%v), got: (%v)", name, expectedErr, actualErr)
+	}
+
+	if !reflect.DeepEqual(actual, expected) {
+		t.Fatalf("%s: expected: (%v), got: (%v)", name, expected, actual)
+	}
+}