Skip to content

Commit b59a380

Browse files
committed
Improve error messages
1 parent 05f4293 commit b59a380

File tree

3 files changed

+32
-26
lines changed

3 files changed

+32
-26
lines changed

router.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ func (r *Router) DELETE(path string, handle Handle) {
213213
// communication with a proxy).
214214
func (r *Router) Handle(method, path string, handle Handle) {
215215
if path[0] != '/' {
216-
panic("path must begin with '/'")
216+
panic("path must begin with '/' in path '" + path + "'")
217217
}
218218

219219
if r.trees == nil {
@@ -257,7 +257,7 @@ func (r *Router) HandlerFunc(method, path string, handler http.HandlerFunc) {
257257
// router.ServeFiles("/src/*filepath", http.Dir("/var/www"))
258258
func (r *Router) ServeFiles(path string, root http.FileSystem) {
259259
if len(path) < 10 || path[len(path)-10:] != "/*filepath" {
260-
panic("path must end with /*filepath")
260+
panic("path must end with /*filepath in path '" + path + "'")
261261
}
262262

263263
fileServer := http.FileServer(root)

tree.go

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package httprouter
77
import (
88
"strings"
99
"unicode"
10-
"fmt"
1110
)
1211

1312
func min(a, b int) int {
@@ -79,6 +78,7 @@ func (n *node) incrementChildPrio(pos int) int {
7978
// addRoute adds a node with the given handle to the path.
8079
// Not concurrency-safe!
8180
func (n *node) addRoute(path string, handle Handle) {
81+
fullPath := path
8282
n.priority++
8383
numParams := countParams(path)
8484

@@ -148,7 +148,9 @@ func (n *node) addRoute(path string, handle Handle) {
148148
}
149149
}
150150

151-
panic(fmt.Sprintf("conflict with wildcard route. has %q got %q", path, n.path))
151+
panic("path segment '" + path +
152+
"' conflicts with existing wildcard '" + n.path +
153+
"' in path '" + fullPath + "'")
152154
}
153155

154156
c := path[0]
@@ -180,23 +182,23 @@ func (n *node) addRoute(path string, handle Handle) {
180182
n.incrementChildPrio(len(n.indices) - 1)
181183
n = child
182184
}
183-
n.insertChild(numParams, path, handle)
185+
n.insertChild(numParams, path, fullPath, handle)
184186
return
185187

186188
} else if i == len(path) { // Make node a (in-path) leaf
187189
if n.handle != nil {
188-
panic(fmt.Sprintf("a Handle is already registered for this path (%q)", path))
190+
panic("a handle is already registered for path ''" + fullPath + "'")
189191
}
190192
n.handle = handle
191193
}
192194
return
193195
}
194196
} else { // Empty tree
195-
n.insertChild(numParams, path, handle)
197+
n.insertChild(numParams, path, fullPath, handle)
196198
}
197199
}
198200

199-
func (n *node) insertChild(numParams uint8, path string, handle Handle) {
201+
func (n *node) insertChild(numParams uint8, path, fullPath string, handle Handle) {
200202
var offset int // already handled bytes of the path
201203

202204
// find prefix until first wildcard (beginning with ':'' or '*'')
@@ -206,27 +208,29 @@ func (n *node) insertChild(numParams uint8, path string, handle Handle) {
206208
continue
207209
}
208210

209-
// check if this Node existing children which would be
210-
// unreachable if we insert the wildcard here
211-
if len(n.children) > 0 {
212-
panic("wildcard route conflicts with existing children")
213-
}
214-
215211
// find wildcard end (either '/' or path end)
216212
end := i + 1
217213
for end < max && path[end] != '/' {
218214
switch path[end] {
219215
// the wildcard name must not contain ':' and '*'
220216
case ':', '*':
221-
panic("only one wildcard per path segment is allowed")
217+
panic("only one wildcard per path segment is allowed, has: '" +
218+
path[i:] + "' in path '" + fullPath + "'")
222219
default:
223220
end++
224221
}
225222
}
226223

224+
// check if this Node existing children which would be
225+
// unreachable if we insert the wildcard here
226+
if len(n.children) > 0 {
227+
panic("wildcard route '" + path[i:end] +
228+
"' conflicts with existing children in path '" + fullPath + "'")
229+
}
230+
227231
// check if the wildcard has a name
228232
if end-i < 2 {
229-
panic("wildcards must be named with a non-empty name")
233+
panic("wildcards must be named with a non-empty name in path '" + fullPath + "'")
230234
}
231235

232236
if c == ':' { // param
@@ -262,17 +266,17 @@ func (n *node) insertChild(numParams uint8, path string, handle Handle) {
262266

263267
} else { // catchAll
264268
if end != max || numParams > 1 {
265-
panic("catch-all routes are only allowed at the end of the path")
269+
panic("catch-all routes are only allowed at the end of the path in path '" + fullPath + "'")
266270
}
267271

268272
if len(n.path) > 0 && n.path[len(n.path)-1] == '/' {
269-
panic("catch-all conflicts with existing handle for the path segment root")
273+
panic("catch-all conflicts with existing handle for the path segment root in path '" + fullPath + "'")
270274
}
271275

272276
// currently fixed width 1 for '/'
273277
i--
274278
if path[i] != '/' {
275-
panic("no / before catch-all")
279+
panic("no / before catch-all in path '" + fullPath + "'")
276280
}
277281

278282
n.path = path[offset:i]
@@ -397,7 +401,7 @@ walk: // Outer loop for walking the tree
397401
return
398402

399403
default:
400-
panic("Invalid node type")
404+
panic("invalid node type")
401405
}
402406
}
403407
} else if path == n.path {
@@ -508,7 +512,7 @@ func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) (ciPa
508512
return append(ciPath, path...), true
509513

510514
default:
511-
panic("Invalid node type")
515+
panic("invalid node type")
512516
}
513517
} else {
514518
// We should have reached the node containing the handle.

tree_test.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ func TestTreeDoubleWildcard(t *testing.T) {
358358
tree.addRoute(route, nil)
359359
})
360360

361-
if rs, ok := recv.(string); !ok || rs != panicMsg {
361+
if rs, ok := recv.(string); !ok || !strings.HasPrefix(rs, panicMsg) {
362362
t.Fatalf(`"Expected panic "%s" for route '%s', got "%v"`, panicMsg, route, recv)
363363
}
364364
}
@@ -584,6 +584,8 @@ func TestTreeFindCaseInsensitivePath(t *testing.T) {
584584
}
585585

586586
func TestTreeInvalidNodeType(t *testing.T) {
587+
const panicMsg = "invalid node type"
588+
587589
tree := &node{}
588590
tree.addRoute("/", fakeHandler("/"))
589591
tree.addRoute("/:page", fakeHandler("/:page"))
@@ -595,15 +597,15 @@ func TestTreeInvalidNodeType(t *testing.T) {
595597
recv := catchPanic(func() {
596598
tree.getValue("/test")
597599
})
598-
if rs, ok := recv.(string); !ok || rs != "Invalid node type" {
599-
t.Fatalf(`Expected panic "Invalid node type", got "%v"`, recv)
600+
if rs, ok := recv.(string); !ok || rs != panicMsg {
601+
t.Fatalf("Expected panic '"+panicMsg+"', got '%v'", recv)
600602
}
601603

602604
// case-insensitive lookup
603605
recv = catchPanic(func() {
604606
tree.findCaseInsensitivePath("/test", true)
605607
})
606-
if rs, ok := recv.(string); !ok || rs != "Invalid node type" {
607-
t.Fatalf(`Expected panic "Invalid node type", got "%v"`, recv)
608+
if rs, ok := recv.(string); !ok || rs != panicMsg {
609+
t.Fatalf("Expected panic '"+panicMsg+"', got '%v'", recv)
608610
}
609611
}

0 commit comments

Comments
 (0)