Merge lp:~rvb/gwacl/not-found-errors into lp:gwacl
- not-found-errors
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Raphaël Badin |
Approved revision: | 124 |
Merged at revision: | 124 |
Proposed branch: | lp:~rvb/gwacl/not-found-errors |
Merge into: | lp:gwacl |
Diff against target: |
674 lines (+236/-119) 4 files modified
httperror.go (+34/-0) httperror_test.go (+38/-0) storage_base.go (+21/-10) storage_base_test.go (+143/-109) |
To merge this branch: | bzr merge lp:~rvb/gwacl/not-found-errors |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella | Approve | ||
Review via email: mp+170792@code.launchpad.net |
Commit message
Propagate Azure errors, add IsNotFoundError() method.
Description of the change
This branch does two things:
- it adds two utility methods:
- extendError(). This method is used to extend the message of different types of error (generic errors and ServerError errors). The goal is to be able to extend errors in a generic fashion without losing the extra information that a more specialized error might carry.
- IsNotFoundError(). This method is a tiny utility that a third party application might use when getting an error back from gwacl to determine if the underlining Azure error is a 404 error. This is needed in the provider and I figured it would be better to provide a utility method rather than to let the caller do the casting and the comparison with a gwacl-specific redefinition of an HTTPStatus (with we have this redefinition, I don't know).
- it use the extendError() method in the methods from storage_base.go to propagate Azure-related error instead of hiding this from the caller.
One more thing: adding makeHttpResponse() is a small improvement I made to the code since creating an http.Response was done over and over again in the test code. It hads a bit of noise to the diff but once I got started and thought "hum, could I land this as a pre-req?" it was already too late… besides, the pipeline plugin does not play well with colocated branches.
This was pre-imp'ed with Jeroen.
Raphaël Badin (rvb) wrote : | # |
> Looks okay, but I want you to consider [1] before I approve.
This looks much too big to be considered *before* this lands to be honest. An interesting idea but clearly outside the scope of this branch; the good thing about this branch is that all the extendError(
A few things about the error chain idea:
- I like the general idea although I'm afraid that it might be flagged as non-idiomatic (whatever that means) because it's basically a slightly awkward implementation of a dynamic traceback being built manually :).
- Such a generic pattern, if a complete analysis prove that it's worth it, should probably live in its own project so that many projects can benefit from it.
Gavin Panella (allenap) wrote : | # |
> > Looks okay, but I want you to consider [1] before I approve.
>
> This looks much too big to be considered *before* this lands to be honest.
Yeah, I guess it's *instead* of parts of this branch. Things like
IsNotFoundError can be reimplemented to use this quite easily.
> An
> interesting idea but clearly outside the scope of this branch; the good thing
> about this branch is that all the extendError(
> well encapsulated so we could move to something like what you describe in the
> future.
I'm not sure it's clearly outside of scope, but okay.
> A few things about the error chain idea:
> - I like the general idea although I'm afraid that it might be flagged as non-
> idiomatic (whatever that means) because it's basically a slightly awkward
> implementation of a dynamic traceback being built manually :).
We're the GWACL maintainers, so we get to say what's idiomatic. If
smooshing strings together for errors is truly what the Go mainstream
call idiomatic then I'm happy to be a freak. I don't think it is
though. They like tracebacks too, they're just having some cognitive
dissonance issues between tracebacks with panic (or what a duck would
call exceptions) and the dogmatic checking and passing of errors by
hand.
> - Such a generic pattern, if a complete analysis prove that it's worth it,
> should probably live in its own project so that many projects can benefit from
> it.
It's only 39 lines ;)
It might be generically useful, but everything has to start somewhere.
Putting it in a sub-package or just a new file is enough to get going.
Raphaël Badin (rvb) wrote : | # |
> Yeah, I guess it's *instead* of parts of this branch. Things like
> IsNotFoundError can be reimplemented to use this quite easily.
> It might be generically useful, but everything has to start somewhere.
> Putting it in a sub-package or just a new file is enough to get going.
Sounds like a good plan. Thanks for unblocking me… I'm happy to think/talk some more about this idea next week.
Preview Diff
1 | === modified file 'httperror.go' |
2 | --- httperror.go 2013-04-02 08:51:00 +0000 |
3 | +++ httperror.go 2013-06-21 11:47:27 +0000 |
4 | @@ -90,3 +90,37 @@ |
5 | Message: outcome.ErrorMessage, |
6 | } |
7 | } |
8 | + |
9 | +// extendError returns an error whos description is the concatenation of |
10 | +// the given message plus the error string from the original error. |
11 | +// It preserves the value of the error types it knows about (currently only |
12 | +// ServerError). |
13 | +// |
14 | +// The main purpose of this method is to offer a unified way to |
15 | +// extend the information present in errors while still not losing the |
16 | +// additioning information present on specific errors gwacl knows out to extend |
17 | +// in a more meaningful way. |
18 | +func extendError(err error, message string) error { |
19 | + switch err := err.(type) { |
20 | + case *ServerError: |
21 | + returnedErr := *err |
22 | + returnedErr.error = fmt.Errorf(message+"%v", err.error) |
23 | + return &returnedErr |
24 | + default: |
25 | + return fmt.Errorf(message+"%v", err) |
26 | + } |
27 | + // This code cannot be reached but Go insists on having a return or a panic |
28 | + // statement at the end of this method. Sigh. |
29 | + panic("invalid extendError state!") |
30 | +} |
31 | + |
32 | +// IsNotFoundError returns whether or not the given error is an error (as |
33 | +// returned by a gwacl method) which corresponds to a 'Not Found' error |
34 | +// returned by Windows Azure. |
35 | +func IsNotFoundError(err error) bool { |
36 | + serverError, ok := err.(*ServerError) |
37 | + if !ok { |
38 | + return false |
39 | + } |
40 | + return serverError.HTTPStatus.StatusCode() == http.StatusNotFound |
41 | +} |
42 | |
43 | === modified file 'httperror_test.go' |
44 | --- httperror_test.go 2013-04-02 06:49:31 +0000 |
45 | +++ httperror_test.go 2013-06-21 11:47:27 +0000 |
46 | @@ -94,3 +94,41 @@ |
47 | c.Check(err.Code, Equals, code) |
48 | c.Check(err.Message, Equals, message) |
49 | } |
50 | + |
51 | +func (suite *httpErrorSuite) TestExtendErrorExtendsGenericError(c *C) { |
52 | + errorString := "an-error" |
53 | + error := fmt.Errorf(errorString) |
54 | + additionalErrorMsg := "additional message" |
55 | + newError := extendError(error, additionalErrorMsg) |
56 | + c.Check(newError.Error(), Equals, fmt.Sprintf("%s%s", additionalErrorMsg, error.Error())) |
57 | +} |
58 | + |
59 | +func (suite *httpErrorSuite) TestExtendErrorExtendsServerError(c *C) { |
60 | + description := "could not talk to server" |
61 | + status := http.StatusGatewayTimeout |
62 | + httpErr := newHTTPError(status, []byte{}, description) |
63 | + |
64 | + httpServerErr, ok := httpErr.(*ServerError) |
65 | + c.Assert(ok, Equals, true) |
66 | + additionalErrorMsg := "additional message" |
67 | + newError := extendError(httpErr, additionalErrorMsg) |
68 | + newServerError, ok := httpErr.(*ServerError) |
69 | + c.Assert(ok, Equals, true) |
70 | + c.Check(newError.Error(), Equals, fmt.Sprintf("%s%s", additionalErrorMsg, httpErr.Error())) |
71 | + c.Check(httpServerErr.HTTPStatus, Equals, newServerError.HTTPStatus) |
72 | +} |
73 | + |
74 | +func (suite *httpErrorSuite) TestIsNotFound(c *C) { |
75 | + var testValues = []struct { |
76 | + err error |
77 | + expectedResult bool |
78 | + }{ |
79 | + {fmt.Errorf("generic error"), false}, |
80 | + {newHTTPError(http.StatusGatewayTimeout, []byte{}, "error"), false}, |
81 | + {newHTTPError(http.StatusOK, []byte{}, ""), false}, |
82 | + {newHTTPError(http.StatusNotFound, []byte{}, "error"), true}, |
83 | + } |
84 | + for _, test := range testValues { |
85 | + c.Check(IsNotFoundError(test.err), Equals, test.expectedResult) |
86 | + } |
87 | +} |
88 | |
89 | === modified file 'storage_base.go' |
90 | --- storage_base.go 2013-06-21 01:01:35 +0000 |
91 | +++ storage_base.go 2013-06-21 11:47:27 +0000 |
92 | @@ -356,7 +356,8 @@ |
93 | ExpectedStatus: http.StatusOK, |
94 | }) |
95 | if err != nil { |
96 | - return nil, fmt.Errorf("request for containers list failed: %v", err) |
97 | + msg := "request for containers list failed: " |
98 | + return nil, extendError(err, msg) |
99 | } |
100 | return &containers, nil |
101 | } |
102 | @@ -392,7 +393,8 @@ |
103 | ExpectedStatus: http.StatusOK, |
104 | }) |
105 | if err != nil { |
106 | - return nil, fmt.Errorf("request for blobs list failed: %v", err) |
107 | + msg := "request for blobs list failed: " |
108 | + return nil, extendError(err, msg) |
109 | } |
110 | return &blobs, err |
111 | } |
112 | @@ -410,7 +412,8 @@ |
113 | ExpectedStatus: http.StatusCreated, |
114 | }) |
115 | if err != nil { |
116 | - return fmt.Errorf("failed to create container %s: %v", container, err) |
117 | + msg := fmt.Sprintf("failed to create container %s: ", container) |
118 | + return extendError(err, msg) |
119 | } |
120 | return nil |
121 | } |
122 | @@ -453,7 +456,8 @@ |
123 | ExpectedStatus: http.StatusCreated, |
124 | }) |
125 | if err != nil { |
126 | - return fmt.Errorf("failed to create blob %s: %v", req.Filename, err) |
127 | + msg := fmt.Sprintf("failed to create blob %s: ", req.Filename) |
128 | + return extendError(err, msg) |
129 | } |
130 | return nil |
131 | } |
132 | @@ -488,7 +492,8 @@ |
133 | ExpectedStatus: http.StatusCreated, |
134 | }) |
135 | if err != nil { |
136 | - return fmt.Errorf("failed to put page for file %s: %v", req.Filename, err) |
137 | + msg := fmt.Sprintf("failed to put page for file %s: ", req.Filename) |
138 | + return extendError(err, msg) |
139 | } |
140 | return nil |
141 | } |
142 | @@ -509,7 +514,8 @@ |
143 | ExpectedStatus: http.StatusOK, |
144 | }) |
145 | if err != nil { |
146 | - return nil, fmt.Errorf("request for block list in file %s failed: %v", filename, err) |
147 | + msg := fmt.Sprintf("request for block list in file %s failed: ", filename) |
148 | + return nil, extendError(err, msg) |
149 | } |
150 | return &bl, nil |
151 | } |
152 | @@ -530,7 +536,8 @@ |
153 | ExpectedStatus: http.StatusCreated, |
154 | }) |
155 | if err != nil { |
156 | - return fmt.Errorf("failed to put block %s for file %s: %v", id, filename, err) |
157 | + msg := fmt.Sprintf("failed to put block %s for file %s: ", id, filename) |
158 | + return extendError(err, msg) |
159 | } |
160 | return nil |
161 | } |
162 | @@ -554,7 +561,8 @@ |
163 | ExpectedStatus: http.StatusCreated, |
164 | }) |
165 | if err != nil { |
166 | - return fmt.Errorf("failed to put blocklist for file %s: %v", filename, err) |
167 | + msg := fmt.Sprintf("failed to put blocklist for file %s: ", filename) |
168 | + return extendError(err, msg) |
169 | } |
170 | return nil |
171 | } |
172 | @@ -568,8 +576,10 @@ |
173 | ExpectedStatus: http.StatusAccepted, |
174 | }) |
175 | // TODO Handle a 404 with an <Error>BlobNotFound response body silently. |
176 | + // Now this is easy to fix with the method IsNotFoundError(). |
177 | if err != nil { |
178 | - return fmt.Errorf("failed to delete blob %s: %v", filename, err) |
179 | + msg := fmt.Sprintf("failed to delete blob %s: ", filename) |
180 | + return extendError(err, msg) |
181 | } |
182 | return nil |
183 | } |
184 | @@ -583,7 +593,8 @@ |
185 | ExpectedStatus: http.StatusOK, |
186 | }) |
187 | if err != nil { |
188 | - return nil, fmt.Errorf("failed to get blob %s: %v", filename, err) |
189 | + msg := fmt.Sprintf("failed to get blob %s: ", filename) |
190 | + return nil, extendError(err, msg) |
191 | } |
192 | return response.Body, nil |
193 | } |
194 | |
195 | === modified file 'storage_base_test.go' |
196 | --- storage_base_test.go 2013-06-21 01:01:35 +0000 |
197 | +++ storage_base_test.go 2013-06-21 11:47:27 +0000 |
198 | @@ -20,6 +20,14 @@ |
199 | |
200 | var _ = Suite(&testComposeHeaders{}) |
201 | |
202 | +func makeHttpResponse(status int, body string) *http.Response { |
203 | + return &http.Response{ |
204 | + Status: fmt.Sprintf("%d", status), |
205 | + StatusCode: status, |
206 | + Body: makeResponseBody(body), |
207 | + } |
208 | +} |
209 | + |
210 | func (suite *testComposeHeaders) TestNoHeaders(c *C) { |
211 | req, err := http.NewRequest("GET", "http://example.com", nil) |
212 | c.Assert(err, IsNil) |
213 | @@ -301,7 +309,7 @@ |
214 | // The ListContainers Storage API call returns a ContainerEnumerationResults |
215 | // struct on success. |
216 | func (suite *TestListContainers) Test(c *C) { |
217 | - response_body := ` |
218 | + responseBody := ` |
219 | <?xml version="1.0" encoding="utf-8"?> |
220 | <EnumerationResults AccountName="http://myaccount.blob.core.windows.net"> |
221 | <Prefix>prefix-value</Prefix> |
222 | @@ -325,11 +333,7 @@ |
223 | </Containers> |
224 | <NextMarker/> |
225 | </EnumerationResults>` |
226 | - response := &http.Response{ |
227 | - Status: fmt.Sprintf("%d", http.StatusOK), |
228 | - StatusCode: http.StatusOK, |
229 | - Body: makeResponseBody(response_body), |
230 | - } |
231 | + response := makeHttpResponse(http.StatusOK, responseBody) |
232 | transport := &TestTransport{Response: response} |
233 | context := makeStorageContext(transport) |
234 | request := &ListContainersRequest{Marker: ""} |
235 | @@ -351,16 +355,16 @@ |
236 | c.Assert(err, NotNil) |
237 | } |
238 | |
239 | -// Server-side errors are propagated back to the caller. |
240 | -func (suite *TestListContainers) TestErrorResponse(c *C) { |
241 | - response := &http.Response{ |
242 | - Status: fmt.Sprintf("%d", http.StatusNotFound), |
243 | - StatusCode: http.StatusNotFound, |
244 | - } |
245 | +// Azure HTTP errors (for instance 404 responses) are propagated back to |
246 | +// the caller as ServerError objects. |
247 | +func (suite *TestListContainers) TestServerError(c *C) { |
248 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
249 | context := makeStorageContext(&TestTransport{Response: response}) |
250 | request := &ListContainersRequest{Marker: ""} |
251 | _, err := context.ListContainers(request) |
252 | - c.Assert(err, NotNil) |
253 | + serverError, ok := err.(*ServerError) |
254 | + c.Check(ok, Equals, true) |
255 | + c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound) |
256 | } |
257 | |
258 | func (suite *TestListContainers) TestListContainersBatchPassesMarker(c *C) { |
259 | @@ -424,7 +428,7 @@ |
260 | // The ListBlobs Storage API call returns a BlobEnumerationResults struct on |
261 | // success. |
262 | func (suite *TestListBlobs) Test(c *C) { |
263 | - response_body := ` |
264 | + responseBody := ` |
265 | <?xml version="1.0" encoding="utf-8"?> |
266 | <EnumerationResults ContainerName="http://myaccount.blob.core.windows.net/mycontainer"> |
267 | <Prefix>prefix</Prefix> |
268 | @@ -468,11 +472,7 @@ |
269 | </Blobs> |
270 | <NextMarker /> |
271 | </EnumerationResults>` |
272 | - response := &http.Response{ |
273 | - Status: fmt.Sprintf("%d", http.StatusOK), |
274 | - StatusCode: http.StatusOK, |
275 | - Body: makeResponseBody(response_body), |
276 | - } |
277 | + response := makeHttpResponse(http.StatusOK, responseBody) |
278 | transport := &TestTransport{Response: response} |
279 | context := makeStorageContext(transport) |
280 | |
281 | @@ -498,17 +498,16 @@ |
282 | c.Assert(err, NotNil) |
283 | } |
284 | |
285 | -// Server-side errors are propagated back to the caller. |
286 | -func (suite *TestListBlobs) TestErrorResponse(c *C) { |
287 | - response := &http.Response{ |
288 | - Status: fmt.Sprintf("%d", http.StatusNotFound), |
289 | - StatusCode: http.StatusNotFound, |
290 | - } |
291 | +// Azure HTTP errors (for instance 404 responses) are propagated back to |
292 | +// the caller as ServerError objects. |
293 | +func (suite *TestListBlobs) TestServerError(c *C) { |
294 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
295 | context := makeStorageContext(&TestTransport{Response: response}) |
296 | - |
297 | request := &ListBlobsRequest{Container: "container"} |
298 | _, err := context.ListBlobs(request) |
299 | - c.Assert(err, NotNil) |
300 | + serverError, ok := err.(*ServerError) |
301 | + c.Check(ok, Equals, true) |
302 | + c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound) |
303 | } |
304 | |
305 | func (suite *TestListBlobs) TestListBlobsPassesMarker(c *C) { |
306 | @@ -592,10 +591,7 @@ |
307 | // The CreateContainer Storage API call returns without error when the |
308 | // container has been created successfully. |
309 | func (suite *TestCreateContainer) Test(c *C) { |
310 | - response := &http.Response{ |
311 | - Status: fmt.Sprintf("%d", http.StatusCreated), |
312 | - StatusCode: http.StatusCreated, |
313 | - } |
314 | + response := makeHttpResponse(http.StatusCreated, "") |
315 | transport := &TestTransport{Response: response} |
316 | context := makeStorageContext(transport) |
317 | container_name := MakeRandomString(10) |
318 | @@ -617,10 +613,7 @@ |
319 | |
320 | // Server-side errors are propagated back to the caller. |
321 | func (suite *TestCreateContainer) TestErrorResponse(c *C) { |
322 | - response := &http.Response{ |
323 | - Status: fmt.Sprintf("%d", http.StatusNotFound), |
324 | - StatusCode: http.StatusNotFound, |
325 | - } |
326 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
327 | context := makeStorageContext(&TestTransport{Response: response}) |
328 | err := context.CreateContainer("container") |
329 | c.Assert(err, NotNil) |
330 | @@ -628,25 +621,30 @@ |
331 | |
332 | // Server-side errors are propagated back to the caller. |
333 | func (suite *TestCreateContainer) TestNotCreatedResponse(c *C) { |
334 | - response := &http.Response{ |
335 | - Status: fmt.Sprintf("%d", http.StatusOK), |
336 | - StatusCode: http.StatusOK, |
337 | - } |
338 | + response := makeHttpResponse(http.StatusOK, "") |
339 | context := makeStorageContext(&TestTransport{Response: response}) |
340 | err := context.CreateContainer("container") |
341 | c.Assert(err, NotNil) |
342 | } |
343 | |
344 | +// Azure HTTP errors (for instance 404 responses) are propagated back to |
345 | +// the caller as ServerError objects. |
346 | +func (suite *TestCreateContainer) TestServerError(c *C) { |
347 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
348 | + context := makeStorageContext(&TestTransport{Response: response}) |
349 | + err := context.CreateContainer("container") |
350 | + serverError, ok := err.(*ServerError) |
351 | + c.Check(ok, Equals, true) |
352 | + c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound) |
353 | +} |
354 | + |
355 | type TestPutPage struct{} |
356 | |
357 | var _ = Suite(&TestPutPage{}) |
358 | |
359 | // Basic happy path testing. |
360 | func (suite *TestPutPage) TestHappyPath(c *C) { |
361 | - response := &http.Response{ |
362 | - Status: fmt.Sprintf("%d", http.StatusCreated), |
363 | - StatusCode: http.StatusCreated, |
364 | - } |
365 | + response := makeHttpResponse(http.StatusCreated, "") |
366 | transport := &TestTransport{Response: response} |
367 | context := makeStorageContext(transport) |
368 | randomData := MakeRandomByteSlice(10) |
369 | @@ -687,11 +685,8 @@ |
370 | |
371 | // Server-side errors are propagated back to the caller. |
372 | func (suite *TestPutPage) TestErrorResponse(c *C) { |
373 | - response := &http.Response{ |
374 | - Status: "102 Frotzed", |
375 | - StatusCode: 102, |
376 | - Body: makeResponseBody("<Error><Code>Frotzed</Code><Message>failed to put blob</Message></Error>"), |
377 | - } |
378 | + responseBody := "<Error><Code>Frotzed</Code><Message>failed to put blob</Message></Error>" |
379 | + response := makeHttpResponse(102, responseBody) |
380 | context := makeStorageContext(&TestTransport{Response: response}) |
381 | err := context.PutPage(&PutPageRequest{ |
382 | Container: "container", Filename: "filename", StartRange: 0, |
383 | @@ -702,16 +697,26 @@ |
384 | c.Check(err, ErrorMatches, ".*failed to put blob.*") |
385 | } |
386 | |
387 | +// Azure HTTP errors (for instance 404 responses) are propagated back to |
388 | +// the caller as ServerError objects. |
389 | +func (suite *TestPutPage) TestServerError(c *C) { |
390 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
391 | + context := makeStorageContext(&TestTransport{Response: response}) |
392 | + err := context.PutPage(&PutPageRequest{ |
393 | + Container: "container", Filename: "filename", StartRange: 0, |
394 | + EndRange: 512, Data: nil}) |
395 | + serverError, ok := err.(*ServerError) |
396 | + c.Check(ok, Equals, true) |
397 | + c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound) |
398 | +} |
399 | + |
400 | type TestPutBlob struct{} |
401 | |
402 | var _ = Suite(&TestPutBlob{}) |
403 | |
404 | // Test basic PutBlob happy path functionality. |
405 | func (suite *TestPutBlob) TestPutBlockBlob(c *C) { |
406 | - response := &http.Response{ |
407 | - Status: fmt.Sprintf("%d", http.StatusCreated), |
408 | - StatusCode: http.StatusCreated, |
409 | - } |
410 | + response := makeHttpResponse(http.StatusCreated, "") |
411 | transport := &TestTransport{Response: response} |
412 | context := makeStorageContext(transport) |
413 | |
414 | @@ -728,10 +733,7 @@ |
415 | |
416 | // PutBlob should set x-ms-blob-type to PageBlob for Page Blobs. |
417 | func (suite *TestPutBlob) TestPutPageBlob(c *C) { |
418 | - response := &http.Response{ |
419 | - Status: fmt.Sprintf("%d", http.StatusCreated), |
420 | - StatusCode: http.StatusCreated, |
421 | - } |
422 | + response := makeHttpResponse(http.StatusCreated, "") |
423 | transport := &TestTransport{Response: response} |
424 | context := makeStorageContext(transport) |
425 | err := context.PutBlob(&PutBlobRequest{ |
426 | @@ -774,11 +776,8 @@ |
427 | |
428 | // Server-side errors are propagated back to the caller. |
429 | func (suite *TestPutBlob) TestErrorResponse(c *C) { |
430 | - response := &http.Response{ |
431 | - Status: "102 Frotzed", |
432 | - StatusCode: 102, |
433 | - Body: makeResponseBody("<Error><Code>Frotzed</Code><Message>failed to put blob</Message></Error>"), |
434 | - } |
435 | + responseBody := "<Error><Code>Frotzed</Code><Message>failed to put blob</Message></Error>" |
436 | + response := makeHttpResponse(102, responseBody) |
437 | context := makeStorageContext(&TestTransport{Response: response}) |
438 | err := context.PutBlob(&PutBlobRequest{ |
439 | Container: "container", BlobType: "block", Filename: "blobname"}) |
440 | @@ -788,15 +787,24 @@ |
441 | c.Check(err, ErrorMatches, ".*failed to put blob.*") |
442 | } |
443 | |
444 | +// Azure HTTP errors (for instance 404 responses) are propagated back to |
445 | +// the caller as ServerError objects. |
446 | +func (suite *TestPutBlob) TestServerError(c *C) { |
447 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
448 | + context := makeStorageContext(&TestTransport{Response: response}) |
449 | + err := context.PutBlob(&PutBlobRequest{ |
450 | + Container: "container", BlobType: "block", Filename: "blobname"}) |
451 | + serverError, ok := err.(*ServerError) |
452 | + c.Check(ok, Equals, true) |
453 | + c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound) |
454 | +} |
455 | + |
456 | type TestPutBlock struct{} |
457 | |
458 | var _ = Suite(&TestPutBlock{}) |
459 | |
460 | func (suite *TestPutBlock) Test(c *C) { |
461 | - response := &http.Response{ |
462 | - Status: fmt.Sprintf("%d", http.StatusCreated), |
463 | - StatusCode: http.StatusCreated, |
464 | - } |
465 | + response := makeHttpResponse(http.StatusCreated, "") |
466 | transport := &TestTransport{Response: response} |
467 | context := makeStorageContext(transport) |
468 | blockid := "\x1b\xea\xf7Mv\xb5\xddH\xebm" |
469 | @@ -830,11 +838,8 @@ |
470 | |
471 | // Server-side errors are propagated back to the caller. |
472 | func (suite *TestPutBlock) TestErrorResponse(c *C) { |
473 | - response := &http.Response{ |
474 | - Status: "102 Frotzed", |
475 | - StatusCode: 102, |
476 | - Body: makeResponseBody("<Error><Code>Frotzed</Code><Message>failed to put block</Message></Error>"), |
477 | - } |
478 | + responseBody := "<Error><Code>Frotzed</Code><Message>failed to put block</Message></Error>" |
479 | + response := makeHttpResponse(102, responseBody) |
480 | context := makeStorageContext(&TestTransport{Response: response}) |
481 | data_reader := bytes.NewReader(MakeRandomByteSlice(10)) |
482 | err := context.PutBlock("container", "blobname", "blockid", data_reader) |
483 | @@ -844,15 +849,24 @@ |
484 | c.Check(err, ErrorMatches, ".*failed to put block.*") |
485 | } |
486 | |
487 | +// Azure HTTP errors (for instance 404 responses) are propagated back to |
488 | +// the caller as ServerError objects. |
489 | +func (suite *TestPutBlock) TestServerError(c *C) { |
490 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
491 | + context := makeStorageContext(&TestTransport{Response: response}) |
492 | + data_reader := bytes.NewReader(MakeRandomByteSlice(10)) |
493 | + err := context.PutBlock("container", "blobname", "blockid", data_reader) |
494 | + serverError, ok := err.(*ServerError) |
495 | + c.Check(ok, Equals, true) |
496 | + c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound) |
497 | +} |
498 | + |
499 | type TestPutBlockList struct{} |
500 | |
501 | var _ = Suite(&TestPutBlockList{}) |
502 | |
503 | func (suite *TestPutBlockList) Test(c *C) { |
504 | - response := &http.Response{ |
505 | - Status: fmt.Sprintf("%d", http.StatusCreated), |
506 | - StatusCode: http.StatusCreated, |
507 | - } |
508 | + response := makeHttpResponse(http.StatusCreated, "") |
509 | transport := &TestTransport{Response: response} |
510 | context := makeStorageContext(transport) |
511 | blocklist := &BlockList{} |
512 | @@ -888,11 +902,8 @@ |
513 | |
514 | // Server-side errors are propagated back to the caller. |
515 | func (suite *TestPutBlockList) TestErrorResponse(c *C) { |
516 | - response := &http.Response{ |
517 | - Status: "102 Frotzed", |
518 | - StatusCode: 102, |
519 | - Body: makeResponseBody("<Error><Code>Frotzed</Code><Message>failed to put blocklist</Message></Error>"), |
520 | - } |
521 | + responseBody := "<Error><Code>Frotzed</Code><Message>failed to put blocklist</Message></Error>" |
522 | + response := makeHttpResponse(102, responseBody) |
523 | context := makeStorageContext(&TestTransport{Response: response}) |
524 | blocklist := &BlockList{} |
525 | err := context.PutBlockList("container", "blobname", blocklist) |
526 | @@ -902,6 +913,18 @@ |
527 | c.Check(err, ErrorMatches, ".*failed to put blocklist.*") |
528 | } |
529 | |
530 | +// Azure HTTP errors (for instance 404 responses) are propagated back to |
531 | +// the caller as ServerError objects. |
532 | +func (suite *TestPutBlockList) TestServerError(c *C) { |
533 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
534 | + context := makeStorageContext(&TestTransport{Response: response}) |
535 | + blocklist := &BlockList{} |
536 | + err := context.PutBlockList("container", "blobname", blocklist) |
537 | + serverError, ok := err.(*ServerError) |
538 | + c.Check(ok, Equals, true) |
539 | + c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound) |
540 | +} |
541 | + |
542 | type TestGetBlockList struct{} |
543 | |
544 | var _ = Suite(&TestGetBlockList{}) |
545 | @@ -909,7 +932,7 @@ |
546 | // The GetBlockList Storage API call returns a GetBlockList struct on |
547 | // success. |
548 | func (suite *TestGetBlockList) Test(c *C) { |
549 | - response_body := ` |
550 | + responseBody := ` |
551 | <?xml version="1.0" encoding="utf-8"?> |
552 | <BlockList> |
553 | <CommittedBlocks> |
554 | @@ -926,11 +949,7 @@ |
555 | </UncommittedBlocks> |
556 | </BlockList>` |
557 | |
558 | - response := &http.Response{ |
559 | - Status: fmt.Sprintf("%d", http.StatusOK), |
560 | - StatusCode: http.StatusOK, |
561 | - Body: makeResponseBody(response_body), |
562 | - } |
563 | + response := makeHttpResponse(http.StatusOK, responseBody) |
564 | transport := &TestTransport{Response: response} |
565 | context := makeStorageContext(transport) |
566 | results, err := context.GetBlockList("container", "myfilename") |
567 | @@ -952,15 +971,15 @@ |
568 | c.Assert(err, NotNil) |
569 | } |
570 | |
571 | -// Server-side errors are propagated back to the caller. |
572 | -func (suite *TestGetBlockList) TestErrorResponse(c *C) { |
573 | - response := &http.Response{ |
574 | - Status: fmt.Sprintf("%d", http.StatusNotFound), |
575 | - StatusCode: http.StatusNotFound, |
576 | - } |
577 | +// Azure HTTP errors (for instance 404 responses) are propagated back to |
578 | +// the caller as ServerError objects. |
579 | +func (suite *TestGetBlockList) TestServerError(c *C) { |
580 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
581 | context := makeStorageContext(&TestTransport{Response: response}) |
582 | - _, err := context.GetBlockList("container", "myfilename") |
583 | - c.Assert(err, NotNil) |
584 | + _, err := context.GetBlockList("container", "blobname") |
585 | + serverError, ok := err.(*ServerError) |
586 | + c.Check(ok, Equals, true) |
587 | + c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound) |
588 | } |
589 | |
590 | type TestDeleteBlob struct{} |
591 | @@ -968,10 +987,7 @@ |
592 | var _ = Suite(&TestDeleteBlob{}) |
593 | |
594 | func (suite *TestDeleteBlob) Test(c *C) { |
595 | - response := &http.Response{ |
596 | - Status: fmt.Sprintf("%d", http.StatusAccepted), |
597 | - StatusCode: http.StatusAccepted, |
598 | - } |
599 | + response := makeHttpResponse(http.StatusAccepted, "") |
600 | transport := &TestTransport{Response: response} |
601 | context := makeStorageContext(transport) |
602 | err := context.DeleteBlob("container", "blobname") |
603 | @@ -991,13 +1007,21 @@ |
604 | c.Assert(err, NotNil) |
605 | } |
606 | |
607 | +// Azure HTTP errors (for instance 404 responses) are propagated back to |
608 | +// the caller as ServerError objects. |
609 | +func (suite *TestDeleteBlob) TestServerError(c *C) { |
610 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
611 | + context := makeStorageContext(&TestTransport{Response: response}) |
612 | + err := context.DeleteBlob("container", "blobname") |
613 | + serverError, ok := err.(*ServerError) |
614 | + c.Check(ok, Equals, true) |
615 | + c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound) |
616 | +} |
617 | + |
618 | // Server-side errors are propagated back to the caller. |
619 | func (suite *TestDeleteBlob) TestErrorResponse(c *C) { |
620 | - response := &http.Response{ |
621 | - Status: "146 Frotzed", |
622 | - StatusCode: 146, |
623 | - Body: makeResponseBody("<Error><Code>Frotzed</Code><Message>failed to delete blob</Message></Error>"), |
624 | - } |
625 | + responseBody := "<Error><Code>Frotzed</Code><Message>failed to delete blob</Message></Error>" |
626 | + response := makeHttpResponse(146, responseBody) |
627 | context := makeStorageContext(&TestTransport{Response: response}) |
628 | err := context.DeleteBlob("container", "blobname") |
629 | c.Assert(err, NotNil) |
630 | @@ -1011,12 +1035,8 @@ |
631 | var _ = Suite(&TestGetBlob{}) |
632 | |
633 | func (suite *TestGetBlob) Test(c *C) { |
634 | - response_body := "blob-in-a-can" |
635 | - response := &http.Response{ |
636 | - Status: fmt.Sprintf("%d", http.StatusOK), |
637 | - StatusCode: http.StatusOK, |
638 | - Body: makeResponseBody(response_body), |
639 | - } |
640 | + responseBody := "blob-in-a-can" |
641 | + response := makeHttpResponse(http.StatusOK, responseBody) |
642 | transport := &TestTransport{Response: response} |
643 | context := makeStorageContext(transport) |
644 | reader, err := context.GetBlob("container", "blobname") |
645 | @@ -1030,7 +1050,7 @@ |
646 | |
647 | data, err := ioutil.ReadAll(reader) |
648 | c.Assert(err, IsNil) |
649 | - c.Check(string(data), Equals, response_body) |
650 | + c.Check(string(data), Equals, responseBody) |
651 | } |
652 | |
653 | // Client-side errors from the HTTP client are propagated back to the caller. |
654 | @@ -1042,6 +1062,20 @@ |
655 | c.Assert(err, NotNil) |
656 | } |
657 | |
658 | +// Azure HTTP errors (for instance 404 responses) are propagated back to |
659 | +// the caller as ServerError objects. |
660 | +func (suite *TestGetBlob) TestServerError(c *C) { |
661 | + response := makeHttpResponse(http.StatusNotFound, "not found") |
662 | + context := makeStorageContext(&TestTransport{Response: response}) |
663 | + reader, err := context.GetBlob("container", "blobname") |
664 | + c.Check(reader, IsNil) |
665 | + c.Assert(err, NotNil) |
666 | + serverError, ok := err.(*ServerError) |
667 | + c.Check(ok, Equals, true) |
668 | + c.Check(serverError.HTTPStatus.StatusCode(), Equals, http.StatusNotFound) |
669 | + c.Check(IsNotFoundError(err), Equals, true) |
670 | +} |
671 | + |
672 | // Server-side errors are propagated back to the caller. |
673 | func (suite *TestGetBlob) TestErrorResponse(c *C) { |
674 | response := &http.Response{ |
Looks okay, but I want you to consider [1] before I approve.
[1]
+func extendError(err error, message string) error {
I found the suggestion that Jeroen mentioned this morning. It's from a
thread called "Strategy for errors in Go". See what you think:
Instead of munging errors into new ones, and so losing type info, we
could store all the errors in a linked-list. I've not given it massive
amounts of thought, but perhaps it's an interesting idea.
type GWACLErrorType interface { Type) GWACLErrorType
Error func() string
Check func(GWACLError
Chain func(Error) GWACLErrorType
This Error
Next GWACLErrorType
}
type GWACLError struct {
This Error
Next GWACLErrorType
}
// Satisfy the Error interface.
func (self *GWACLError) Error() string {
if self.Next == nil {
return self.Error()
} else {
return self.Error() + "; " + self.Next.Error()
}
panic("Being for the benefit of Go < 1.1.")
}
// Search for an error in the list. Sadly, types are not first-class
// citizens, so this just does equality testing for now, but perhaps
// something better could be dreamt up.
func (self *GWACLError) Check(err *GWACLErrorType) {
for f := self; f = f.Next(); f != nil {
if f == err {
return f
}
}
return nil
}
// Return a new GWACLErrorType with the current error at the head.
func (self *GWACLError) Chain(err Error) GWACLErrorType {
return &GWACLError{err, self}
}
[2]
+ var testValues = []struct {
+ err error
+ expectedResult bool
+ }{
...
+ }
I wasn't aware of this form. Nice to know.