Skip to content

Commit 0ed9bc4

Browse files
committed
Revert "Merge pull request #1 from infobloxopen/master"
This reverts commit 8c8311f, reversing changes made to e18a59a.
1 parent 8c8311f commit 0ed9bc4

File tree

9 files changed

+163
-234
lines changed

9 files changed

+163
-234
lines changed

gateway/errors.go

Lines changed: 12 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,12 @@ package gateway
22

33
import (
44
"context"
5-
"encoding/json"
65
"fmt"
76
"io"
8-
"math"
97
"net/http"
10-
"strconv"
11-
"strings"
12-
"sync/atomic"
13-
"time"
148

15-
"google.golang.org/grpc"
169
"google.golang.org/grpc/codes"
1710
"google.golang.org/grpc/grpclog"
18-
"google.golang.org/grpc/metadata"
1911
"google.golang.org/grpc/status"
2012

2113
"github.com/grpc-ecosystem/grpc-gateway/runtime"
@@ -28,8 +20,12 @@ import (
2820
// Addition bool argument indicates whether method (http.ResponseWriter.WriteHeader) was called or not.
2921
type ProtoStreamErrorHandlerFunc func(context.Context, bool, *runtime.ServeMux, runtime.Marshaler, http.ResponseWriter, *http.Request, error)
3022

31-
type RestErrs struct {
32-
Error []map[string]interface{} `json:"error,omitempty"`
23+
// RestError represents an error in accordance with REST API Syntax Specification.
24+
// See: https://github.com/infobloxopen/atlas-app-toolkit#errors
25+
type RestError struct {
26+
Status *RestStatus `json:"error,omitempty"`
27+
Details []interface{} `json:"details,omitempty"`
28+
Fields interface{} `json:"fields,omitempty"`
3329
}
3430

3531
var (
@@ -110,28 +106,19 @@ func (h *ProtoErrorHandler) writeError(ctx context.Context, headerWritten bool,
110106
}
111107
}
112108

113-
restErr := map[string]interface{}{
114-
"message": st.Message(),
115-
}
116-
if len(details) > 0 {
117-
restErr["details"] = details
118-
}
119-
if fields != nil {
120-
restErr["fields"] = fields
109+
restErr := &RestError{
110+
Status: Status(ctx, st),
111+
Details: details,
112+
Fields: fields,
121113
}
122114

123-
errs, _ := errorsAndSuccessFromContext(ctx)
124-
restResp := &RestErrs{
125-
Error: errs,
126-
}
127-
restResp.Error = append(restResp.Error, restErr)
128115
if !headerWritten {
129116
rw.Header().Del("Trailer")
130117
rw.Header().Set("Content-Type", marshaler.ContentType())
131-
rw.WriteHeader(HTTPStatus(ctx, st))
118+
rw.WriteHeader(restErr.Status.HTTPStatus)
132119
}
133120

134-
buf, merr := marshaler.Marshal(restResp)
121+
buf, merr := marshaler.Marshal(restErr)
135122
if merr != nil {
136123
grpclog.Infof("error handler: failed to marshal error message %q: %v", restErr, merr)
137124
rw.WriteHeader(http.StatusInternalServerError)
@@ -146,125 +133,3 @@ func (h *ProtoErrorHandler) writeError(ctx context.Context, headerWritten bool,
146133
grpclog.Infof("error handler: failed to write response: %v", err)
147134
}
148135
}
149-
150-
// For small performance bump, switch map[string]string to a tuple-type (string, string)
151-
152-
type MessageWithFields interface {
153-
error
154-
GetFields() map[string]interface{}
155-
GetMessage() string
156-
}
157-
type messageWithFields struct {
158-
message string
159-
fields map[string]interface{}
160-
}
161-
162-
func (m *messageWithFields) Error() string {
163-
return m.message
164-
}
165-
func (m *messageWithFields) GetFields() map[string]interface{} {
166-
return m.fields
167-
}
168-
func (m *messageWithFields) GetMessage() string {
169-
return m.message
170-
}
171-
172-
// NewWithFields returns a new MessageWithFields that requires a message string,
173-
// and then treats the following arguments as alternating keys and values
174-
// a non-string key will immediately return the result so far, ignoring later
175-
// values. The values can be any type
176-
func NewWithFields(message string, kvpairs ...interface{}) MessageWithFields {
177-
mwf := &messageWithFields{message: message, fields: make(map[string]interface{})}
178-
for i := 0; i+1 < len(kvpairs); i += 2 {
179-
k, ok := kvpairs[i].(string)
180-
if !ok {
181-
return mwf
182-
}
183-
mwf.fields[k] = kvpairs[i+1]
184-
}
185-
return mwf
186-
}
187-
188-
// For giving each error a unique metadata key, but not leaking the exact count
189-
// of errors or something like that
190-
var counter *uint32
191-
192-
func init() {
193-
counter = new(uint32)
194-
*counter = uint32(time.Now().Nanosecond() % math.MaxUint32)
195-
}
196-
197-
// WithError will save an error message into the grpc trailer metadata, if it
198-
// is an error that implements MessageWithFields, it also saves the fields.
199-
// This error will then be inserted into the return JSON if the ResponseForwarder
200-
// is used
201-
func WithError(ctx context.Context, err error) {
202-
i := atomic.AddUint32(counter, uint32(time.Now().Nanosecond()%100+1))
203-
md := metadata.Pairs(fmt.Sprintf("error-%d", i), fmt.Sprintf("message:%s", err.Error()))
204-
if mwf, ok := err.(MessageWithFields); ok {
205-
if f := mwf.GetFields(); f != nil {
206-
b, _ := json.Marshal(mwf.GetFields())
207-
md.Append(fmt.Sprintf("error-%d", i), fmt.Sprintf("fields:%q", b))
208-
}
209-
}
210-
grpc.SetTrailer(ctx, md)
211-
}
212-
213-
// WithSuccess will save a MessageWithFields into the grpc trailer metadata.
214-
// This success message will then be inserted into the return JSON if the
215-
// ResponseForwarder is used
216-
func WithSuccess(ctx context.Context, msg MessageWithFields) {
217-
i := atomic.AddUint32(counter, uint32(time.Now().Nanosecond()%100+1))
218-
md := metadata.Pairs(fmt.Sprintf("success-%d", i), fmt.Sprintf("message:%s", msg.Error()))
219-
if f := msg.GetFields(); f != nil {
220-
b, _ := json.Marshal(msg.GetFields())
221-
md.Append(fmt.Sprintf("success-%d", i), fmt.Sprintf("fields:%q", b))
222-
}
223-
grpc.SetTrailer(ctx, md)
224-
}
225-
226-
func errorsAndSuccessFromContext(ctx context.Context) (errors []map[string]interface{}, success map[string]interface{}) {
227-
md, ok := runtime.ServerMetadataFromContext(ctx)
228-
if !ok {
229-
return nil, nil
230-
}
231-
errors = make([]map[string]interface{}, 0)
232-
latestSuccess := int64(-1)
233-
for k, vs := range md.TrailerMD {
234-
if strings.HasPrefix(k, "error-") {
235-
err := make(map[string]interface{})
236-
for _, v := range vs {
237-
parts := strings.SplitN(v, ":", 2)
238-
if parts[0] == "fields" {
239-
uq, _ := strconv.Unquote(parts[1])
240-
json.Unmarshal([]byte(uq), &err)
241-
} else if parts[0] == "message" {
242-
err["message"] = parts[1]
243-
}
244-
}
245-
errors = append(errors, err)
246-
}
247-
if num := strings.TrimPrefix(k, "success-"); num != k {
248-
// Let the later success messages override previous ones,
249-
// also account for the possiblity of wraparound with a generous check
250-
if i, err := strconv.ParseInt(num, 10, 32); err == nil {
251-
if i > latestSuccess || (i < 1<<12 && latestSuccess > 1<<28) {
252-
latestSuccess = i
253-
} else {
254-
continue
255-
}
256-
}
257-
success = make(map[string]interface{})
258-
for _, v := range vs {
259-
parts := strings.SplitN(v, ":", 2)
260-
if parts[0] == "fields" {
261-
uq, _ := strconv.Unquote(parts[1])
262-
json.Unmarshal([]byte(uq), &success)
263-
} else if parts[0] == "message" {
264-
success["message"] = parts[1]
265-
}
266-
}
267-
}
268-
}
269-
return
270-
}

gateway/errors_test.go

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,14 @@ import (
1212
"github.com/infobloxopen/atlas-app-toolkit/errors"
1313

1414
"github.com/grpc-ecosystem/grpc-gateway/runtime"
15+
"google.golang.org/genproto/googleapis/rpc/code"
1516
"google.golang.org/grpc/codes"
1617
"google.golang.org/grpc/status"
1718
)
1819

1920
func TestProtoMessageErrorHandlerUnknownCode(t *testing.T) {
2021
err := fmt.Errorf("simple text error")
21-
v := &RestErrs{}
22+
v := new(RestError)
2223

2324
rw := httptest.NewRecorder()
2425
ProtoMessageErrorHandler(context.Background(), nil, &runtime.JSONBuiltin{}, rw, nil, err)
@@ -34,14 +35,22 @@ func TestProtoMessageErrorHandlerUnknownCode(t *testing.T) {
3435
t.Fatalf("failed to unmarshal response: %s", err)
3536
}
3637

37-
if v.Error[0]["message"] != "simple text error" {
38-
t.Errorf("invalid message: %s", v.Error[0]["message"])
38+
if v.Status.HTTPStatus != http.StatusInternalServerError {
39+
t.Errorf("invalid http status: %d", v.Status.HTTPStatus)
40+
}
41+
42+
if v.Status.Code != code.Code_UNKNOWN.String() {
43+
t.Errorf("invalid code: %s", v.Status.Code)
44+
}
45+
46+
if v.Status.Message != "simple text error" {
47+
t.Errorf("invalid message: %s", v.Status.Message)
3948
}
4049
}
4150

4251
func TestProtoMessageErrorHandlerUnimplementedCode(t *testing.T) {
4352
err := status.Error(codes.Unimplemented, "service not implemented")
44-
v := new(RestErrs)
53+
v := new(RestError)
4554

4655
rw := httptest.NewRecorder()
4756
ProtoMessageErrorHandler(context.Background(), nil, &runtime.JSONBuiltin{}, rw, nil, err)
@@ -57,8 +66,16 @@ func TestProtoMessageErrorHandlerUnimplementedCode(t *testing.T) {
5766
t.Fatalf("failed to unmarshal response: %s", err)
5867
}
5968

60-
if v.Error[0]["message"] != "service not implemented" {
61-
t.Errorf("invalid message: %s", v.Error[0]["message"])
69+
if v.Status.HTTPStatus != http.StatusNotImplemented {
70+
t.Errorf("invalid http status: %d", v.Status.HTTPStatus)
71+
}
72+
73+
if v.Status.Code != "NOT_IMPLEMENTED" {
74+
t.Errorf("invalid code: %s", v.Status.Code)
75+
}
76+
77+
if v.Status.Message != "service not implemented" {
78+
t.Errorf("invalid message: %s", v.Status.Message)
6279
}
6380
}
6481

@@ -69,7 +86,7 @@ func TestWriteErrorContainer(t *testing.T) {
6986
WithDetail(codes.AlreadyExists, "resource", "x btw already exists.").
7087
WithField("x", "Check correct value of 'x'.")
7188

72-
v := new(RestErrs)
89+
v := new(RestError)
7390

7491
rw := httptest.NewRecorder()
7592
ProtoMessageErrorHandler(context.Background(), nil, &runtime.JSONBuiltin{}, rw, nil, err)
@@ -78,12 +95,20 @@ func TestWriteErrorContainer(t *testing.T) {
7895
t.Fatalf("failed to unmarshal response: %s", err)
7996
}
8097

81-
if v.Error[0]["message"] != "Invalid 'x' value." {
82-
t.Errorf("invalid message: %s", v.Error[0]["message"])
98+
if v.Status.HTTPStatus != http.StatusBadRequest {
99+
t.Errorf("invalid http status: %d", v.Status.HTTPStatus)
100+
}
101+
102+
if v.Status.Code != "INVALID_ARGUMENT" {
103+
t.Errorf("invalid code: %s", v.Status.Code)
104+
}
105+
106+
if v.Status.Message != "Invalid 'x' value." {
107+
t.Errorf("invalid message: %s", v.Status.Message)
83108
}
84109

85-
if len(v.Error[0]["details"].([]interface{})) != 2 {
86-
t.Errorf("invalid details length: %d", len(v.Error[0]["details"].([]interface{})))
110+
if len(v.Details) != 2 {
111+
t.Errorf("invalid details length: %d", len(v.Details))
87112
}
88113

89114
details := []interface{}{
@@ -100,19 +125,19 @@ func TestWriteErrorContainer(t *testing.T) {
100125
}
101126

102127
if !reflect.DeepEqual(
103-
v.Error[0]["details"],
128+
v.Details,
104129
details,
105130
) {
106-
t.Errorf("invalid details value: %v", v.Error[0]["details"])
131+
t.Errorf("invalid details value: %v", v.Details)
107132
}
108133

109134
fields := map[string][]string{
110135
"x": []string{"Check correct value of 'x'."}}
111136

112-
vMap := v.Error[0]["fields"].(map[string]interface{})
137+
vMap := v.Fields.(map[string]interface{})
113138

114139
if vMap["x"].([]interface{})[0] != fields["x"][0] {
115-
t.Errorf("invalid fields value: %v", v.Error[0]["fields"])
140+
t.Errorf("invalid fields value: %v", v.Fields)
116141
}
117142

118143
}

gateway/header.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,6 @@ func handleForwardResponseServerMetadata(matcher runtime.HeaderMatcherFunc, w ht
100100

101101
func handleForwardResponseTrailerHeader(w http.ResponseWriter, md runtime.ServerMetadata) {
102102
for k := range md.TrailerMD {
103-
if strings.HasPrefix(k, "error-") || strings.HasPrefix(k, "success-") {
104-
continue
105-
}
106103
tKey := textproto.CanonicalMIMEHeaderKey(fmt.Sprintf("%s%s", runtime.MetadataTrailerPrefix, k))
107104
w.Header().Add("Trailer", tKey)
108105
}

0 commit comments

Comments
 (0)