Ignore non-spec-compliant errors

[kibbles-n-bytes]

Currently, we expect error responses to be able to be marshaled into the following:

type failureResponseBody struct {
        Err         *string `json:"error,omitempty"`
        Description *string `json:"description,omitempty"`
}

The spec says we should ignore errors that are not of the expected format. However, responses that match these keys but are not strings but instead nested objects will cause an unmarshaling error, which creates a HTTPStatusCodeError object that looks like the following:

Status: 500; ErrorMessage: <nil>; Description:
      <nil>; ResponseError: json: cannot unmarshal object into Go struct field failureResponseBody.error
      of type string

This error is surfaced to the user when it should be ignored, and is misleading as the error reported here is not from the broker but from the go client.

[kibbles-n-bytes]

Reported by @bmelville on the Kubernetes-Incubator Service Catalog here: https://github.com/kubernetes-incubator/service-catalog/issues/1290

[kibbles-n-bytes]

Looking into it further, right now the HTTPStatusCodeError type has the following definition:

type HTTPStatusCodeError struct {
        // StatusCode is the HTTP status code returned by the broker.
        StatusCode int 
        // ErrorMessage is a machine-readable error string that may be returned by
        // the broker.
        ErrorMessage *string
        // Description is a human-readable description of the error that may be
        // returned by the broker.
        Description *string
        // ResponseError is set to the error that occurred when unmarshalling a
        // response body from the broker.
        ResponseError error
}

The spec says that non-conformant errors should be completely ignored, so in that case the only field here relevant to the spec is Description. The simplest thing would be to remove ErrorMessage and ResponseError. Unmarshal errors would be surfaced purely as an empty Description, as that is what the spec dictates. If users want to see more detail, they can turn on Verbose on their client and look at the logs.

[fejta-bot]

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten