diff --git a/changelog.markdown b/changelog.markdown index ca5e2d9..0a77639 100644 --- a/changelog.markdown +++ b/changelog.markdown @@ -3,6 +3,8 @@ ## 0.6.0 - ??/07/2021 - **Breaking change:** the `@` operator used for retrieving request parameters now automatically decodes special characters using `decodeUrl`. +- Fix for [#211](https://github.com/dom96/jester/issues/211) - custom routers now have the same error handling as normal routes. +- Fix for [#269](https://github.com/dom96/jester/issues/269) - a bug that prevented redirecting from within error handlers. ## 0.5.0 - 17/10/2020 diff --git a/jester.nim b/jester.nim index 1a6abd6..9cebf8e 100644 --- a/jester.nim +++ b/jester.nim @@ -42,6 +42,14 @@ type request: Request, error: RouteError ): Future[ResponseData] {.gcsafe, closure.} + MatchPair* = tuple + matcher: MatchProc + errorHandler: ErrorProc + + MatchPairSync* = tuple + matcher: MatchProcSync + errorHandler: ErrorProc + Jester* = object when not useHttpBeast: httpServer*: AsyncHttpServer @@ -98,6 +106,10 @@ proc unsafeSend(request: Request, content: string) = # TODO: This may cause issues if we send too fast. asyncCheck request.getNativeReq.client.send(content) +proc newCompletedFuture(): Future[void] = + result = newFuture[void]() + complete(result) + proc send( request: Request, code: HttpCode, headers: Option[RawHeaders], body: string ): Future[void] = @@ -106,9 +118,7 @@ proc send( if headers.isNone: "" else: headers.get().createHeaders request.getNativeReq.send(code, body, h) - var fut = newFuture[void]() - complete(fut) - return fut + return newCompletedFuture() else: return request.getNativeReq.respond( code, body, newHttpHeaders(headers.get(@({:}))) @@ -121,6 +131,7 @@ proc statusContent(request: Request, status: HttpCode, content: string, when not defined(release): logging.debug(" $1 $2" % [$status, toStr(headers)]) except: + result = newCompletedFuture() logging.error("Could not send response: $1" % osErrorMsg(osLastError())) # TODO: Add support for proper Future Streams instead of this weird raw mode. @@ -450,6 +461,22 @@ proc initJester*( result.matchers = @[] result.errorHandlers = @[] +proc initJester*( + pair: MatchPair, + settings: Settings = newSettings() +): Jester = + result = initJester(settings) + result.register(pair.matcher) + result.register(pair.errorHandler) + +proc initJester*( + pair: MatchPairSync, # TODO: Annoying nim bug: `MatchPair | MatchPairSync` doesn't work. + settings: Settings = newSettings() +): Jester = + result = initJester(settings) + result.register(pair.matcher) + result.register(pair.errorHandler) + proc initJester*( matcher: MatchProc, settings: Settings = newSettings() @@ -458,7 +485,7 @@ proc initJester*( result.register(matcher) proc initJester*( - matcher: MatchProcSync, # TODO: Annoying nim bug: `MatchProc | MatchProcSync` doesn't work. + matcher: MatchProcSync, settings: Settings = newSettings() ): Jester = result = initJester(settings) @@ -515,7 +542,7 @@ proc serve*( runForever() template setHeader*(headers: var ResponseHeaders, key, value: string): typed = - ## Sets a response header using the given key and value. + ## Sets a response header using the given key and value. ## Overwrites if the header key already exists. bind isNone if isNone(headers): @@ -1303,7 +1330,7 @@ proc routesEx(name: string, body: NimNode): NimNode = `afterRoutes` ) - let matchIdent = newIdentNode(name) + let matchIdent = newIdentNode(name & "Matcher") let reqIdent = newIdentNode("request") let needsAsync = needsAsync(body) case needsAsync @@ -1338,31 +1365,49 @@ proc routesEx(name: string, body: NimNode): NimNode = # Error handler proc let errorHandlerIdent = newIdentNode(name & "ErrorHandler") let errorIdent = newIdentNode("error") - let exceptionIdent = newIdentNode("exception") - let resultIdent = newIdentNode("result") - var errorHandlerProc = quote do: - proc `errorHandlerIdent`( - `reqIdent`: Request, `errorIdent`: RouteError - ): Future[ResponseData] {.gcsafe, async.} = - block `routesListIdent`: - `setDefaultRespIdent`() - case `errorIdent`.kind - of RouteException: - discard - of RouteCode: - discard + let allRoutesIdent = ident("allRoutes") + var exceptionStmts = newStmtList() if exceptionBranches.len != 0: - var stmts = newStmtList() for branch in exceptionBranches: - stmts.add(newIfStmt(branch)) - errorHandlerProc[6][0][1][^1][1][1][0] = stmts + exceptionStmts.add(newIfStmt(branch)) + var codeStmts = newStmtList() if httpCodeBranches.len != 0: - var stmts = newStmtList() for branch in httpCodeBranches: - stmts.add(newIfStmt(branch)) - errorHandlerProc[6][0][1][^1][2][1][0] = stmts + codeStmts.add(newIfStmt(branch)) + var errorHandlerProc = quote do: + proc `errorHandlerIdent`( + `reqIdent`: Request, `errorIdent`: RouteError + ): Future[ResponseData] {.gcsafe, async.} = + block `allRoutesIdent`: + block `routesListIdent`: + `setDefaultRespIdent`() + case `errorIdent`.kind + of RouteException: + `exceptionStmts` + of RouteCode: + `codeStmts` result.add(errorHandlerProc) + # Pair the matcher and error matcher + let pairIdent = newIdentNode(name) + let matchProcVarIdent = newIdentNode(name & "MatchProc") + let errorProcVarIdent = newIdentNode(name & "ErrorProc") + if needsAsync in {ImplicitTrue, ExplicitTrue}: + # TODO: I don't understand why I have to assign these procs to intermediate + # variables in order to get them into the tuple. It would be nice if it could + # just be: + # let `pairIdent`: MatchPair = (`matchIdent`, `errorHandlerIdent`) + result.add quote do: + let `matchProcVarIdent`: MatchProc = `matchIdent` + let `errorProcVarIdent`: ErrorProc = `errorHandlerIdent` + let `pairIdent`: MatchPair = (`matchProcVarIdent`, `errorProcVarIdent`) + else: + result.add quote do: + let `matchProcVarIdent`: MatchProcSync = `matchIdent` + let `errorProcVarIdent`: ErrorProc = `errorHandlerIdent` + let `pairIdent`: MatchPairSync = (`matchProcVarIdent`, `errorProcVarIdent`) + + # TODO: Replace `body`, `headers`, `code` in routes with `result[i]` to # get these shortcuts back without sacrificing usability. # TODO2: Make sure you replace what `guessAction` used to do for this. @@ -1373,13 +1418,11 @@ proc routesEx(name: string, body: NimNode): NimNode = macro routes*(body: untyped) = result = routesEx("match", body) let jesIdent = genSym(nskVar, "jes") - let matchIdent = newIdentNode("match") - let errorHandlerIdent = newIdentNode("matchErrorHandler") + let pairIdent = newIdentNode("match") let settingsIdent = newIdentNode("settings") result.add( quote do: - var `jesIdent` = initJester(`matchIdent`, `settingsIdent`) - `jesIdent`.register(`errorHandlerIdent`) + var `jesIdent` = initJester(`pairIdent`, `settingsIdent`) ) result.add( quote do: diff --git a/tests/customRouter.nim b/tests/customRouter.nim new file mode 100644 index 0000000..e007231 --- /dev/null +++ b/tests/customRouter.nim @@ -0,0 +1,25 @@ +import jester + +router myrouter: + get "/": + resp "Hello world" + + get "/404": + resp "you got 404" + + get "/raise": + raise newException(Exception, "Foobar") + + error Exception: + resp Http500, "Something bad happened: " & exception.msg + + error Http404: + redirect uri("/404") + +when isMainModule: + let s = newSettings( + Port(5454), + bindAddr="127.0.0.1", + ) + var jest = initJester(myrouter, s) + jest.serve() diff --git a/tests/issue296.nim b/tests/issue296.nim new file mode 100644 index 0000000..88bf0f3 --- /dev/null +++ b/tests/issue296.nim @@ -0,0 +1,12 @@ +# Note, this isn't ran as part of the test suite as it relies on randomness too much. + +import jester, asyncdispatch, random, logging + +setLogFilter(lvlInfo) +routes: + before "/": + setLogFilter(lvlInfo) + get "/": + let dur = rand(2000) + await sleepAsync(dur) + resp "hi" \ No newline at end of file diff --git a/tests/tester.nim b/tests/tester.nim index c2222a4..c41c13b 100644 --- a/tests/tester.nim +++ b/tests/tester.nim @@ -253,12 +253,32 @@ proc issue150(useStdLib: bool) = check resp.code == Http500 check (waitFor resp.body).startsWith("Something bad happened") +proc customRouterTest(useStdLib: bool) = + waitFor startServer("customRouter.nim", useStdLib) + var client = newAsyncHttpClient(maxRedirects = 0) + + suite "customRouter useStdLib=" & $useStdLib: + test "error handler": + let resp = waitFor client.get(address & "/raise") + check resp.code == Http500 + let body = (waitFor resp.body) + checkpoint body + check body.startsWith("Something bad happened: Foobar") + + test "redirect in error": + let resp = waitFor client.get(address & "/definitely404route") + check resp.code == Http303 + check resp.headers["location"] == address & "/404" + check (waitFor resp.body) == "" + when isMainModule: try: allTest(useStdLib=false) # Test HttpBeast. allTest(useStdLib=true) # Test asynchttpserver. issue150(useStdLib=false) issue150(useStdLib=true) + customRouterTest(useStdLib=false) + customRouterTest(useStdLib=true) # Verify that Nim in Action Tweeter still compiles. test "Nim in Action - Tweeter":