follow

#21 WebEvents initialized incorrectly

Reported by mnlipp 2011-08-22 (last edited unknown)

It is possible to create more than one http server in an application by using different channel parameter values when creating the components. Each of these correctly creates its HTTP components, passing on the channel parameter and things work until the request is read.

When this happens, HTTP._on_read pushes a Request event. While this event's target seems to be set to the HTTP component's channel (showing up in the debugger as "Request[special-web:request]") all the accompanying events (success, failure, filter, start, end) are targeted at "web".

The reason is that Request (and all other web events) use class attributes for _target (?), success, failure, filter, start and end. While _target seems to be modified somehow (I'll probably be in trouble when my requests start coming in from the different servers concurrently), the others are initialized once and kept unmodified.

Consequently, the request is never answered by my server, because request_success for my HTTP component never happens (with the correct target).

I think the web events should use instance attributes for those attributes because you can have more than one "ductwork" for web related events in your application.

Attachments

Comments and changes

1. #1 James Mills

   written 2011-08-22

   (Reply via prol...@shortcircuit.net.au):

   Hmmm nice spotting! Seems like a bug :) It should work!

   Will fix ASAP!

   cheers James

   James Mills / prologic

   Developer | circuits, sahriswiki E: prologic@shortcircuit.net.au S: therealprologic W: softcircuit.com.au

2. #2 Alessio Deiana

   written 2011-08-28

   I've written a test and changed web events to use an instance target. While that works and channels/targets are respected, now we an issue when registering Controllers. The Dispatcher on_register, unregisters the added controllers but when you have 2 dispatchers that does not play nicely.

3. #3 mnlipp

   written 2011-08-28

   • Added attachment dispatcher.py.

   Right. I have replaced the default dispatcher with my own (I didn't mention it because this can be done without changing existing code, it's just my own new dispatcher).

   What I don't like about the standard controller is that it moves the controller components in the hierarchy as a side effect. Some components in my application that answer to web requests aren't controllers in the first place. They are components with a specific purpose (determining their place in the hierarchy) that also provide a web representation of their state (but more as an additional feature).

   So I have overridden the _on_[un]registered methods of the default dispatcher to only add/remove the components' handlers (instead of the components). To be precise, only the components' handlers that have a target that starts with "/xxx" are added or removed, with "/xxx" being set when initializing the dispatcher (using the dispatcher's channel as xxx).

   Works great for me.

4. #4 James Mills

   written 2011-12-10

   Has this issue been fixed in the deve branch?

   --James

5. #5 mnlipp

   written 22 days ago

   No ist hasn't. Very simple test case: modify controller.py to look like this:

   from circuits.web import Server, Controller, Logger
   from circuits.core.debugger import Debugger
   
   class Root(Controller):
   
       def index(self):
           return "Hello World!"
   
   (Server(("0.0.0.0", 8000), channel="subWeb") + Debugger() + Root() + Logger()).run()
   

   You would expect the server to use channel "subWeb" now and it does. So things seem to work at first. But eventually, the HTTP component waits for the request_success event to appear (on the proper channel). But the "accompanying" events aren't adjusted properly and are therefore still delivered to channel "web", as can be seen from the output.

   <Registered[subWeb.registered] (<TCPServer/subWeb 21420:MainThread (queued=0) [S]>, <Server/subWeb 21420:MainThread (queued=0) [R]> )>
   <Registered[subWeb.registered] (<HTTP/subWeb 21420:MainThread (queued=0) [S]>, <Server/subWeb 21420:MainThread (queued=2) [R]> )>
   <Registered[subWeb.registered] (<Dispatcher/subWeb 21420:MainThread (queued=0) [S]>, <Server/subWeb 21420:MainThread (queued=2) [R]> )>
   <Registered[*.registered] (<Debugger/* 21420:MainThread (queued=0) [S]>, <Server/subWeb 21420:MainThread (queued=2) [R]> )>
   <Registered[/.registered] (<Root// 21420:MainThread (queued=0) [S]>, <Server/subWeb 21420:MainThread (queued=2) [R]> )>
   <Registered[web.registered] (<Logger/web 21420:MainThread (queued=0) [S]>, <Server/subWeb 21420:MainThread (queued=2) [R]> )>
   <Started[subWeb.started] (<Server/subWeb 21420:MainThread (queued=2) [R]> )>
   <Registered[select.registered] (<Select/select 21420:MainThread (queued=0) [S]>, <TCPServer/subWeb 21420:MainThread (queued=0) [S]> )>
   <Ready[subWeb.ready] (<TCPServer/subWeb 21420:MainThread (queued=0) [S]> )>
   <Read[subWeb._read] (<socket._socketobject object at 0x9941064> )>
   <Connect[subWeb.connect] (<socket._socketobject object at 0x994187c>, '127.0.0.1', 53917 )>
   <Read[subWeb._read] (<socket._socketobject object at 0x9941064> )>
   <Read[subWeb._read] (<socket._socketobject object at 0x994187c> )>
   <Connect[subWeb.connect] (<socket._socketobject object at 0x994195c>, '127.0.0.1', 53918 )>
   <Read[subWeb.read] (<socket._socketobject object at 0x994187c>, 'GET / HTTP/1.1\r\nHost: localhost:8000\r\nConnection: keep-alive\r\nCache-Control: max-age=0\r\nUser-Agent: Mozilla/5.0 (X11; Linux i686) AppleWebKit/535.2 (KHTML, like Gecko) Ubuntu/11.10 Chromium/15.0.874.106 Chrome/15.0.874.106 Safari/535.2\r\nAccept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8\r\nAccept-Encoding: gzip,deflate,sdch\r\nAccept-Language: de-DE,de;q=0.8,en-US;q=0.6,en;q=0.4\r\nAccept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3\r\n\r\n' )>
   <Request[subWeb.request] (<Request GET / HTTP/1.1>, <Response 200 OK text/html; charset=utf-8 (0)> )>
   <RequestSuccess[web.request_success] (<Index[<Root// 21420:MainThread (queued=0) [S]>.index] (<Request GET / HTTP/1.1>, <Response 200 OK text/html; charset=utf-8 (0)> )> )>
   <Read[subWeb._read] (<socket._socketobject object at 0x994195c> )>
   <Disconnect[subWeb.disconnect] (<socket._socketobject object at 0x994195c> )>
   

   Now as the HTTP component (also having the proper channel "subWeb") waits forever for subWeb.request_success, the client never gets an answer.

   I fixed this issue (bad hack) for 1.6 by fixing circuits using this code

   # Fix circuits problems
   def fix_circuits():
       _orig_fireEvent = Manager.fireEvent
       def _fix_fired_request (self, event, channel=None, target=None):
           if isinstance(event, Request) \
               and not getattr(event, "_has_been_fixed", False):
               event.success = "request_success", self.channel
               event.failure = "request_failure", self.channel
               event.filter = "request_filtered", self.channel
               event.start = "request_started", self.channel
               event.end = "request_completed", self.channel
               event._has_been_fixed = True
           return _orig_fireEvent (self, event, channel, target)
       Manager.fireEvent = _fix_fired_request
       Manager.fire = Manager.fireEvent
   

   Of course, this is not the "proper" way to do it, just my workaround that allows me to leave the circuits library untouched.

   -- Michael

6. #6 James Mills

   written 21 days ago

   (Reply via prol...@shortcircuit.net.au):

   Thanks for the report! I'll check this out and get it fixed soon.

   cheers James

   James Mills / prologic

   Developer | circuits, sahriswiki E: prologic@shortcircuit.net.au S: therealprologic W: softcircuit.com.au

7. #7 mnlipp

   written 19 days ago

   • Added attachment circuits-dev-8554409aaf.diff.

   Here's a patch that fixes the problem for the development branch. Adding a property to the Request event like I do is not the best way to handle this, but I'm not sure at this point what matches circuits' style best.

   Note that this patch doesn't allow you to create more than one web server, it is only a prerequisite for doing this. I have an additional component in my project that does this (a dispatcher "bound" to a channel). But of course this requires the possibility to have two properly working Server instances with different channels in the first place.

8. #8 James Mills

   written 19 days ago

   (Reply via prol...@shortcircuit.net.au):

   Thanks for your efforts! I'll have a look tonight and apply your patch if I can!

   Thanks ;)

   cheers James

   Sent from my iPad

9. #9 James Mills

   written 19 days ago

   • Changed status from new to open.
   • Added attachment issue21.diff.

   Please see the attached diff against revision f33bf566f25b

   I believe this is the correct fix as it takes advantage of one of circuits core features: being able to raise events upon success or failure of an event handler.

   Note how there are attributes on the WebEvent class in circuits/web/events.py ? These automatically trigger

   • Success or *Failure events of the Request event for circuits.web and fixes your problem.

   The diff attached against the revision above fixes your issue - mosts of the web tests pass, there are a few broken tests but I think/believe they're unrelated or can be fixed easily.

   --JamesMills / prologic

10. #10 James Mills

    written 19 days ago

    prologic@daisy
    Fri Feb 03 21:37:32
    ~/circuits-dev
    $ hg bisect -g
    The first bad revision is:
    changeset:   2910:63cab9855360
    user:        Alessio Deiana <adeiana@gmail.com>
    date:        Fri Jul 15 20:35:39 2011 +0200
    summary:     fixes web/test_core.py test_simple, rename request_success in generate_response and it is now fire from the Controller once request_success is fired, this commit also binds on each controller the success/failure handlers for generating a response properly once the requested handler has been executed instead of loosing the event because it was fired as <channel>.<handler>_success instead of web.<handler>_success
    

    This is the revision that broke this.

    I'll need to look into it more closely.

    --JamesMills / prologic

11. #11 James Mills

    written 19 days ago

    • Changed status from open to resolved.

    The tip of circuits-dev now fixes this issue.

    --JamesMills / prologic

12. #12 mnlipp

    written 18 days ago, last edited 17 days ago

    • Changed status from resolved to open.

    It' almost there. Using RequestSuccess is the better approach, of course. One problem remains, however.

    The "%Success" is fired in Manager without specifying any channels. In my case I have a "top-level" Manager instance, so effectively channel becomes "*" (Manager having no channel member). This means that the event is delivered to all HTTP components in my application (remember, I'm having more than one, that was what started this all). Of course, it should only be delivered to the HTTP-component that initially sent the "Request".

    I can work around this using a filter:

        @handler("request_success", channel="*", filter=True)
        def _on_request_success(self, event, e):
            if getattr(event, "_retargeted", False):
                return False
            setattr(event, "_retargeted", True)
            self.fire(event, *e.channels)
            return event.value
    

    But, I'm wondering if derived "%Success" events shouldn't be sent to the channels of the original event only in general. So the section in Manager._dispatcher would become

            if error is None and event.success:
                self.fire(Success.create("%sSuccess" %
                    event.__class__.__name__, event), *event.channels)
    
    

    (Notice the added *event.channels.)

13. #13 James Mills

    written 12 days ago

    I think I agree with this.

    I'll apply the necessary changes to the core and see if it breaks anything, if not you'll have your wish :)

    cheers
    James

14. #14 James Mills

    written 12 days ago

    • Changed status from open to resolved.

    Fixed in the dev branch.

    cheers
    James

15. #15 mnlipp

    written 12 days ago

    • Added attachment ScopeServers.py.
    • Added attachment dispatcher.py.

    Great. BTW if anybody' interested, here's my ScopeDispatcher and the test case.

16. #16 James Mills

    written 11 days ago

    Minding lodging this as a new ticket, a feature request and briefly describing what it dose, etc?

    I'll close this ticket as done :)

    cheers
    James

17. #17 James Mills

    written 11 days ago

    Actually hold that thought. Just had a quick look.

    The behavior of your ScopeDispatcher and end results are much like the VirtualHostDispatcher for hosting multiple sites on a single server.

    What added benefits does a Scoped Dispatcher provide that VirtualHosts doesn't?

    cheers
    James

18. #18 mnlipp

    written 11 days ago

    You need something like the scoped dispatcher if you want to have two (or more) independant webservers in your application. E.g. I have a web server that handles SOAP/HTTP-requests on one port (port number being prescribed by the specification that I'm implementing, not requiring authentication) and a second web server that provides a small management portal for the application (port chosen by me, probably going to have authentication requirements).

    I assume that I could also create two socket components for the two (web) ports, handle all requests using the standard "web" channel, and separate (dispatch) them again later based on the server port (deriving a virtual host from the server port instead of from the "Host" header in a new dispatcher component). But this is not the "natural" way to do it. Besides I want the small management portal component to be easily reusable in other applications and it should therefore be able to simply create ist own server component, dispatcher(s) and controllers.

    Personally, I think it is much more intuitive to dispatch the events from the two servers to different channels (though I need those "scoped channels") than to have prefixes in the channel name. Given my example, I'd have to use a "virtual host" prefix in all controllers even when using the small management portal for an application that does not provide other web services.

    This is not a feature request. I opened the issue because circuits (then) prevented me from implementing this "my way" and I considered this an issue against the implementation of the architecture that I like very much (circuits is IMHO a great open framework). It's a kind of test case on the architectural level, adding a use case that the architect didn't think about initially and find out if it can nevertheless be implemented using that architecure. Of course, I don't mind if you like the approch and integrate it, but that wasn't my initial intention.

19. #19 James Mills

    written 11 days ago

    (Reply via prol...@shortcircuit.net.au):

    Okay. You've convinced me.

    Please submit a new feature request along with your patches. Feel free to fork and submit a pull request - against the dev branch if possible.

    cheers James

    James Mills / prologic

    Developer | circuits, sahriswiki E: prologic@shortcircuit.net.au S: therealprologic W: softcircuit.com.au

Verification: Please write the text from the image in the box (letters only)

Is that you, Humanoid? Is this me?

• Cancel