Skip to content

Commit ec31f78

Browse files
authored
Refactor/errors (#210)
1 parent 640df04 commit ec31f78

14 files changed

Lines changed: 350 additions & 95 deletions

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ See updating [Changelog example here](https://keepachangelog.com/en/1.0.0/)
55

66
## [Unreleased]
77

8+
### Changed
9+
- errors: all service method now return `Problem` type in case of errors (*BREAKING CHANGE*)
10+
11+
### Removed
12+
- errors: `Error` type was removed (*BREAKING CHANGE*)
13+
814
## [5.4.0]
915

1016
### Added

README.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,14 @@ _, err := svc.GetAccount(context.Background())
7070
// Handle errors in general
7171
if (err != nil) {
7272
// Handle service errors specifically
73-
if serviceError, ok := err.(*upcloud.Error); ok {
74-
fmt.Println(serviceError.ErrorCode)
75-
fmt.Println(serviceError.ErrorMessage)
73+
if serviceError, ok := err.(*upcloud.Problem); ok {
74+
fmt.Println(serviceError.Type)
75+
fmt.Println(serviceError.Title)
76+
fmt.Prinln(serviceError.Status)
77+
// You can use ErrorCode() method to compare error against a set of ErrCode constants
78+
if serviceError.ErrorCode == upcloud.ErrCodeAuthenticationFailed {
79+
// ...
80+
}
7681
}
7782
}
7883
````

upcloud/error.go

Lines changed: 0 additions & 37 deletions
This file was deleted.

upcloud/error_codes.go

Lines changed: 205 additions & 0 deletions
Large diffs are not rendered by default.

upcloud/error_test.go

Lines changed: 0 additions & 26 deletions
This file was deleted.

upcloud/problem.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package upcloud
22

33
import (
44
"fmt"
5+
"net/url"
56
"strings"
67
)
78

@@ -14,12 +15,19 @@ type Problem struct {
1415
// InvalidParams if set, is a list of ProblemInvalidParam describing a specific part(s) of the request
1516
// that caused the problem
1617
InvalidParams []ProblemInvalidParam `json:"invalid_params,omitempty"`
17-
// CorrelationID is an unique string that identifies the request that caused the problem
18+
// CorrelationID is a unique string that identifies the request that caused the problem
19+
// Please note that it is not always available
1820
CorrelationID string `json:"correlation_id,omitempty"`
1921
// HTTP Status code
2022
Status int `json:"status"`
2123
}
2224

25+
// ProblemInvalidParam is a type describing extra information in the Problem type's InvalidParams field.
26+
type ProblemInvalidParam struct {
27+
Name string `json:"name"`
28+
Reason string `json:"reason"`
29+
}
30+
2331
func (p *Problem) Error() string {
2432
var sb strings.Builder
2533
_, _ = fmt.Fprintf(&sb, "error: message=%q, type=%q", p.Title, p.Type)
@@ -34,8 +42,15 @@ func (p *Problem) Error() string {
3442
return sb.String()
3543
}
3644

37-
// ProblemInvalidParam is a type describing extra information in the Problem type's InvalidParams field.
38-
type ProblemInvalidParam struct {
39-
Name string `json:"name"`
40-
Reason string `json:"reason"`
45+
// ErrorCode returns a short string that identifies the error; it should be used for programmatic comparisons
46+
func (p *Problem) ErrorCode() string {
47+
// First check if the type is a URL.
48+
// If it is - we need to extract meaningful fragment from it for comparison purposes
49+
// If it isn't - we can just return the value of `Type` field
50+
parsedURL, err := url.Parse(p.Type)
51+
if err != nil || parsedURL.Scheme == "" || parsedURL.Host == "" {
52+
return p.Type
53+
}
54+
55+
return strings.Replace(parsedURL.Fragment, "ERROR_", "", 1)
4156
}

upcloud/problem_test.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"github.com/stretchr/testify/assert"
88
)
99

10-
func TestProblem(t *testing.T) {
10+
func TestProblemUnmarshal(t *testing.T) {
1111
p := Problem{}
1212
err := json.Unmarshal([]byte(`
1313
{
@@ -31,3 +31,24 @@ func TestProblem(t *testing.T) {
3131
assert.Equal(t, "Backend doesn't exist.", p.InvalidParams[0].Reason)
3232
assert.Equal(t, "default_backend", p.InvalidParams[0].Name)
3333
}
34+
35+
func TestProblemErrorCodes(t *testing.T) {
36+
p := Problem{
37+
Type: "https://api.upcloud.com/1.3/errors#ERROR_RESOURCE_ALREADY_EXISTS",
38+
}
39+
assert.Equal(t, ErrCodeResourceAlreadyExists, p.ErrorCode())
40+
assert.NotEqual(t, ErrCodeAuthenticationFailed, p.ErrorCode())
41+
42+
p.Type = "https://api.upcloud.com/1.3/errors#ERROR_AUTHENTICATION_FAILED"
43+
assert.Equal(t, ErrCodeAuthenticationFailed, p.ErrorCode())
44+
45+
p.Type = "http://api.upcloud.com/1.3/errors#ERROR_INVALID_REQUEST"
46+
assert.Equal(t, ErrCodeInvalidRequest, p.ErrorCode())
47+
48+
p.Type = "GROUP_NOT_FOUND"
49+
assert.Equal(t, ErrCodeGroupNotFound, p.ErrorCode())
50+
51+
p.Type = "SERVER_NOT_FOUND"
52+
assert.Equal(t, ErrCodeServerNotFound, p.ErrorCode())
53+
assert.NotEqual(t, "SOME_RANDOM_STRING", p.ErrorCode())
54+
}

upcloud/service/fixtures/errorhandling.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ interactions:
1010
Content-Type:
1111
- application/json
1212
User-Agent:
13-
- upcloud-go-api/4.9.0
13+
- upcloud-go-api/5.4.0
1414
url: https://api.upcloud.com/1.3/server/invalid/start
1515
method: POST
1616
response:
@@ -27,7 +27,7 @@ interactions:
2727
Content-Type:
2828
- application/json; charset=UTF-8
2929
Date:
30-
- Mon, 24 Oct 2022 11:06:08 GMT
30+
- Tue, 07 Feb 2023 08:05:53 GMT
3131
Server:
3232
- Apache
3333
Strict-Transport-Security:

upcloud/service/managed_database_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,15 +288,15 @@ func TestService_GetManagedDatabaseConnections(t *testing.T) {
288288
Terminate: true,
289289
})
290290
assert.Error(t, err)
291-
assert.True(t, strings.HasPrefix(err.(*upcloud.Error).ErrorMessage, "Must provide a connection"))
291+
assert.True(t, strings.HasPrefix(err.(*upcloud.Problem).Title, "Must provide a connection"))
292292

293293
err = svc.CancelManagedDatabaseConnection(ctx, &request.CancelManagedDatabaseConnection{
294294
UUID: serviceDetails.UUID,
295295
Pid: 0,
296296
Terminate: false,
297297
})
298298
assert.Error(t, err)
299-
assert.True(t, strings.HasPrefix(err.(*upcloud.Error).ErrorMessage, "Must provide a connection"))
299+
assert.True(t, strings.HasPrefix(err.(*upcloud.Problem).Title, "Must provide a connection"))
300300
})
301301
}
302302

upcloud/service/server_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ func TestErrorHandling(t *testing.T) {
223223
})
224224

225225
// Check that the correct error type is returned
226-
expectedErrorType := "*upcloud.Error"
226+
expectedErrorType := "*upcloud.Problem"
227227
actualErrorType := reflect.TypeOf(err).String()
228228

229229
if actualErrorType != expectedErrorType {

0 commit comments

Comments
 (0)