Skip to content

Commit c7767d7

Browse files
axelsavizkyaxelsavizky
authored
improved error messages (#100)
1 parent 15968c2 commit c7767d7

File tree

4 files changed

+67
-37
lines changed

4 files changed

+67
-37
lines changed

‎math/math.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func Swap[T any](x, y *T) {
118118
// Factorial calculates the factorial of a non-negative integer x.
119119
func Factorial(x int) (int, error) {
120120
if x < 0 {
121-
return x, errors.New("Factorial of a negative number is undefined")
121+
return x, errors.New("factorial of a negative number is undefined")
122122
}
123123

124124
if x == 0 || x == 1 {

‎time/time.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func GetMonthName(monthNumber int) (string, error) {
156156
}
157157

158158
if monthNumber < 1 || monthNumber > 12 {
159-
return "", fmt.Errorf("invalid month number: %d", monthNumber)
159+
return "", fmt.Errorf("invalid month number %d - it should be between 1 and 12", monthNumber)
160160
}
161161

162162
return months[monthNumber], nil
@@ -175,7 +175,7 @@ func GetDayName(dayNumber int) (string, error) {
175175
}
176176

177177
if dayNumber < 0 || dayNumber > 6 {
178-
return "", fmt.Errorf("invalid day number: %d", dayNumber)
178+
return "", fmt.Errorf("invalid day number %d - it should be between 0 and 6", dayNumber)
179179
}
180180

181181
return days[dayNumber], nil

‎url/url.go‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,31 @@ package url
55

66
import (
77
"errors"
8+
"fmt"
89
"net/url"
910
"regexp"
1011
"strings"
1112

1213
"golang.org/x/net/publicsuffix"
1314
)
1415

16+
// alphaNumericRegex validates strings containing only letters, numbers, and hyphens
17+
const alphaNumericRegex = "^[a-zA-Z0-9-]+$"
18+
19+
// hostRegex validates hostnames according to RFC 1123
20+
// Rules:
21+
// - Each label must start and end with alphanumeric characters
22+
// - Middle characters can be alphanumeric or hyphens
23+
// - Multiple labels can be joined with dots
24+
const hostRegex = `^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])(\.[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])*$`
25+
26+
// pathRegex validates URL paths containing letters, numbers, hyphens, underscores and forward slashes
27+
const pathRegex = "^[a-zA-Z0-9_-]+(?:/[a-zA-Z0-9_-]+)*$"
28+
29+
var alphaNumericRe = regexp.MustCompile(alphaNumericRegex)
30+
var hostRe = regexp.MustCompile(hostRegex)
31+
var pathRe = regexp.MustCompile(pathRegex)
32+
1533
// BuildURL constructs a URL by combining a scheme, host, path, and query parameters.
1634
//
1735
// Parameters:
@@ -64,8 +82,7 @@ func BuildURL(scheme, host, path string, query map[string]string) (string, error
6482
errMessage = append(errMessage, "scheme is required")
6583
}
6684
if host != "" {
67-
re := regexp.MustCompile(`^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])(\.[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])*$`)
68-
if !re.MatchString(host) {
85+
if !hostRe.MatchString(host) {
6986
errMessage = append(errMessage, "the host is not valid")
7087
}
7188
}
@@ -74,9 +91,8 @@ func BuildURL(scheme, host, path string, query map[string]string) (string, error
7491
}
7592

7693
if path != "" {
77-
re := regexp.MustCompile("^[a-zA-Z]+(\\/[a-zA-Z]+)*$")
78-
if !re.MatchString(path) {
79-
errMessage = append(errMessage, "path is permitted with a-z character and multiple path segments")
94+
if !pathRe.MatchString(path) {
95+
errMessage = append(errMessage, "path is permitted with a-z, 0-9, - and _ characters and multiple path segments")
8096
}
8197
}
8298

@@ -149,25 +165,43 @@ func BuildURL(scheme, host, path string, query map[string]string) (string, error
149165
func AddQueryParams(urlStr string, params map[string]string) (string, error) {
150166
parsedURL, err := url.Parse(urlStr)
151167
if err != nil {
152-
return "", errors.New("URL could not be parsed")
168+
return "", fmt.Errorf("URL %s could not be parsed. err: %w", urlStr, err)
153169
}
154170
switch parsedURL.Scheme {
155171
case "http", "https", "ws", "wss", "ftp":
156172
default:
157-
return "", errors.New("invalid URL scheme")
173+
return "", fmt.Errorf("URL scheme %s is invalid", parsedURL.Scheme)
158174
}
159175
queryParams := parsedURL.Query()
176+
160177
for key, value := range params {
161-
re := regexp.MustCompile("^[a-zA-Z0-9-]+$")
162-
if !re.MatchString(value) || !re.MatchString(key) || value == "" {
163-
return "", errors.New("the query parameter is not valid")
178+
err = validateKeyValue(key, value)
179+
if err != nil {
180+
return "", err
164181
}
165182
queryParams.Add(key, value)
166183
}
167184
parsedURL.RawQuery = queryParams.Encode()
168185
return parsedURL.String(), nil
169186
}
170187

188+
func validateKeyValue(key, value string) error {
189+
if key == "" {
190+
return errors.New("query parameter key cannot be empty")
191+
}
192+
if value == "" {
193+
return fmt.Errorf("query parameter value for key %s cannot be empty", key)
194+
}
195+
if !alphaNumericRe.MatchString(key) {
196+
return fmt.Errorf("query parameter key %s must be alphanumeric", key)
197+
}
198+
if !alphaNumericRe.MatchString(value) {
199+
return fmt.Errorf("query parameter value %s for key %s must be alphanumeric", value, key)
200+
}
201+
202+
return nil
203+
}
204+
171205
// IsValidURL checks whether a given URL string is valid and its scheme matches the allowed list.
172206
//
173207
// Parameters:
@@ -258,16 +292,16 @@ func IsValidURL(urlStr string, allowedReqSchemes []string) bool {
258292
func ExtractDomain(urlStr string) (string, error) {
259293
parsedURL, err := url.Parse(urlStr)
260294
if err != nil {
261-
return "", errors.New("URL could not be parsed")
295+
return "", fmt.Errorf("URL %s could not be parsed. err: %w", urlStr, err)
262296
}
263297

264298
host, err := publicsuffix.EffectiveTLDPlusOne(parsedURL.Hostname())
265299
if err != nil {
266-
return "", errors.New("could not extract public suffix")
300+
return "", fmt.Errorf("could not extract public suffix from host %s. err: %w", parsedURL.Hostname(), err)
267301
}
268302

269303
if host == "" {
270-
return "", errors.New("parameter not found")
304+
return "", errors.New("public suffix is empty")
271305
}
272306

273307
return host, nil
@@ -318,7 +352,7 @@ func GetQueryParam(urlStr, param string) (string, error) {
318352
value, exists := queryParams[param]
319353

320354
if !exists || len(value) == 0 {
321-
return "", errors.New("parameter not found")
355+
return "", fmt.Errorf("parameter %s not found in URL %s", param, urlStr)
322356
}
323357

324358
return value[0], nil

‎url/url_test.go‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ Package url defines url utilities helpers.
33
*/
44
package url
55

6-
import "testing"
6+
import (
7+
"testing"
8+
)
79

810
func TestBuildURL(t *testing.T) {
911
type args struct {
@@ -69,8 +71,8 @@ func TestBuildURLError(t *testing.T) {
6971
},
7072
{
7173
name: "error - build URL",
72-
args: args{scheme: "http", host: "example.com", path: "one1Path2", queryParams: map[string]string{"queryParamOne": "valueQueryParamOne", "queryParamTwo": "valueQueryParamTwo"}},
73-
want: "path is permitted with a-z character and multiple path segments",
74+
args: args{scheme: "http", host: "example.com", path: "one1Path2@", queryParams: map[string]string{"queryParamOne": "valueQueryParamOne", "queryParamTwo": "valueQueryParamTwo"}},
75+
want: "path is permitted with a-z, 0-9, - and _ characters and multiple path segments",
7476
},
7577
{
7678
name: "error - build URL",
@@ -144,22 +146,22 @@ func TestAddQueryParamsError(t *testing.T) {
144146
{
145147
name: "error - add query params",
146148
args: args{urlStr: "htt@p://example.com", queryParams: map[string]string{"queryParamOne": "valueQueryParamOne"}},
147-
want: "URL could not be parsed",
149+
want: "URL htt@p://example.com could not be parsed. err: parse \"htt@p://example.com\": first path segment in URL cannot contain colon",
148150
},
149151
{
150152
name: "error - add query params",
151153
args: args{urlStr: "http://subdomain.example.com", queryParams: map[string]string{"queryParam@One": "valueQueryParamOne", "queryParamTwo": "valueQueryParamTwo"}},
152-
want: "the query parameter is not valid",
154+
want: "query parameter key queryParam@One must be alphanumeric",
153155
},
154156
{
155157
name: "error - add query params",
156158
args: args{urlStr: "http://subdomain.example.com", queryParams: map[string]string{"queryParamOne": "valueQuery@ParamOne", "queryParamTwo": "valueQueryParamTwo"}},
157-
want: "the query parameter is not valid",
159+
want: "query parameter value valueQuery@ParamOne for key queryParamOne must be alphanumeric",
158160
},
159161
{
160162
name: "error - add query params",
161163
args: args{urlStr: "http://subdomain.example.com", queryParams: map[string]string{"queryParamOne": ""}},
162-
want: "the query parameter is not valid",
164+
want: "query parameter value for key queryParamOne cannot be empty",
163165
},
164166
}
165167

@@ -298,7 +300,7 @@ func TestExtractDomainError(t *testing.T) {
298300
{
299301
name: "success - domain with multiple subdomains",
300302
args: args{urlStr: "htt@ps://exam@ple.com"},
301-
want: "URL could not be parsed",
303+
want: "URL htt@ps://exam@ple.com could not be parsed. err: parse \"htt@ps://exam@ple.com\": first path segment in URL cannot contain colon",
302304
},
303305
}
304306

@@ -396,17 +398,11 @@ func TestGetQueryParamError(t *testing.T) {
396398
}{
397399
name: "success - get query param with error",
398400
args: args{urlStr: "http://someurl.com?paramOne=oneValue&paramTwo=otherValue", param: "none"},
399-
want: "parameter not found",
401+
want: "parameter none not found in URL http://someurl.com?paramOne=oneValue&paramTwo=otherValue",
402+
}
403+
if _, err := GetQueryParam(testError.args.urlStr, testError.args.param); err == nil {
404+
t.Errorf("GetQueryParam() expected error, got nil")
405+
} else if err.Error() != testError.want {
406+
t.Errorf("GetQueryParam() error = %v, want %v", err.Error(), testError.want)
400407
}
401-
t.Run(testError.name, func(t *testing.T) {
402-
if _, err := GetQueryParam(testError.args.urlStr, testError.args.param); err == nil {
403-
t.Errorf("GetQueryParam() did not return an error")
404-
}
405-
})
406-
407-
t.Run(testError.name, func(t *testing.T) {
408-
if _, err := GetQueryParam(testError.args.urlStr, testError.args.param); err.Error() != "parameter not found" {
409-
t.Errorf("GetQueryParam() did not return an error")
410-
}
411-
})
412408
}

0 commit comments

Comments
 (0)