orthecreedence / wookie

Asynchronous HTTP server in common lisp
http://wookie.lyonbros.com/
MIT License
189 stars 19 forks source link

Wookie hangs #70

Closed nightshade427 closed 9 years ago

nightshade427 commented 9 years ago

repo:

(ql:quickload "wookie")                                                                                                           
To load "wookie":                                                                                                                          
  Load 1 ASDF system:                                                                                                                      
    wookie                                                                                                                                 
; Loading "wookie"                                                                                                                         
......................                                                                                                                     
("wookie")                                                                                                                                 
CL-USER> (as:with-event-loop (:catch-app-errors t)                                                                                         
             (wookie:start-server (make-instance 'wookie:listener :port 80)))  

Then:

curl "http://www.mydomain.com/test?mine=test this"

This never returns or gives error. It looks like it might be error in fast-http, but the error doesnt bubble up and return anything to client, just hangs :(

orthecreedence commented 9 years ago

I'm guessing this is caused by trying to bind port 80 (unless you're running as root) while also passing :catch-app-errors t. Error handling in cl-async has changed a lot recently, and I haven't had a chance to really update the docs or write an announcement. Can you set :catch-app-errors nil and see what happens?

The short version is catch-app-errors takes a callback of 1 arg now (the error), and here's the long version. Mainly, cl-async no longer catches errors and requires the app itself to do this, allowing for more granular error handling.

nightshade427 commented 9 years ago

It does this with any port. It seems there is an error when using query string params "hi there" instead of "hi+there" in maybe fast-http that isn't bubbling up and is hanging the request?

orthecreedence commented 9 years ago

ah ok, i'll focus my testing on that

nightshade427 commented 9 years ago

Awesome thanks :)

Let me know if you need any more info.

orthecreedence commented 9 years ago

Ah I know why!

By default Wookie catches errors and passes them to our (in this case, nonexistent) error handler. To get it do debug, do a

(let ((wookie-config:*debug-on-error* t))
  ;; start server here
  )

Looks like the problem is in fast-http, I'm assuming that space is making it unable to parse the main HTTP line.

https://github.com/fukamachi/fast-http/issues/18

nightshade427 commented 9 years ago

Its fine if fast-http throws error as I don't think space is valid in Uris. The issue I was having is that wookie just hung and didn't return a response to client like it does for other errors. ("There was an error processing your request")

orthecreedence commented 9 years ago

Ok that makes sense. So in general (and I need to document this probably for all three projects) for a debugging setup that shows you errors with full traces, you want:

(let ((blackbird:*debug-on-error* t)
      (wookie-config:*debug-on-error* t))
  (as:with-event-loop (:catch-app-errors nil)
    ...))

This means all three projects will not fire their error handlers for errors they see and instead let them go into the debugger (you'll still be able to run any restarts) without tampering with the stack traces.

When in production, you'd set all *debug-on-error*s to nil (the default) and do (as:with-event-loop (:catch-app-errors (lambda (err) ...)) ...) and do (make-instance 'wookie:listener :event-cb (lambda (err sock) ...)). This forces all events/errors into the given callbacks (no bubbling up to the debugger).

Sorry for the error handling changes and lack of docs. I'll update these when I can.

nightshade427 commented 9 years ago

Can this error just trigger the regular default error handler and return "there was an error processing your request" like all other errors do? That seems like easiest route as it requires no changes in client code and would behave like rest of system when an unhandled error occures.

orthecreedence commented 9 years ago

Good question.

orthecreedence commented 9 years ago

Ok, fixed in 19c2ccf

If there's an error parsing a client request HTTP, it responds with a 400 error to the client.

nightshade427 commented 9 years ago

This is working great!! Thanks! ;)