collective / collective.taskqueue

Asyncore-based asynchronous task queue for Plone
https://pypi.org/project/collective.taskqueue
8 stars 7 forks source link

Keep SERVER_URL header #20

Open gforcada opened 8 years ago

gforcada commented 8 years ago

collective.taskqueue saves some headers on the request that queues a job so that later triggering the view that is being queued is as close as possible to the original request.

I found a strange error though, the event handler that queues the view reports:

(Pdb) from zope.globalrequest import getRequest
(Pdb) getRequest()['SERVER_URL']
'https://www.devtest.freitag-verlag.de'

While the view that got queued then reports:

(Pdb) from zope.globalrequest import getRequest
(Pdb) getRequest()['SERVER_URL']
'http://10.100.0.205'

I noticed that because on one of the async views that I have I'm sending emails and on its body I put the URL of an article, and instead of being https://www.freitag.de/autoren/der-freitag/RANDOM-ARTCILE is https://www.freitag.de/website/autoren/der-freitag/RANDOM-ARTCILE, notice the website on it, that's the Plone instance id. I'm just getting an object and calling its absolute_url method (that's a Dexterity Container that ends up calling HTTPRequest.physicalPathFromURL and puts the SERVER_URL).

I tried forcing the header by putting it while queuing the job, i.e

taskqueue.add(view_path, headers={'SERVER_URL': getRequest()['SERVER_URL']})

But somehow even though the header is saved correctly, when the task is picked up again it's discarded and http://10.100.0.205 shows up again.

@datakurre am I doing something wrong or that's a valid bug?

gforcada commented 8 years ago

Giving a second look, forcing SERVERURL header it's not discarded, but instead is prefixed with `HTTP` :-/

datakurre commented 8 years ago

Overall that it might be a bug. I need to check, what in Zope2 originally sets SERVERURL. HTTP-prefixing is required for request.get_header (or was it getHeader) and I thought that having headers in that form is enough.

Gil Forcada Codinachs notifications@github.com kirjoitti 24.2.2016 kello 14.49:

Giving a second look, forcing SERVERURL header it's not discarded, but instead is prefixed with HTTP :-/

— Reply to this email directly or view it on GitHub.

datakurre commented 8 years ago

@gforcada Well, I think, this is not a bug after all, but a missing feature. It's not really about passing SERVER_URL to task request, but missing implicit support for VHM (VirtualHostMonster).

You probably see HTTP_SERVER_URL only when you explicitly add it as a header (and that's how headers are stored in request).

You probably have reverse proxy before Plone to prefix incoming request paths with a VHM path. VHM sets SERVER_URL for that request. That information is lost, when you queue task for bare /website/something.

You should be able to test that by prefixing your task path with VHM path to make it something like /VirtualHostBase/https/www.freitag.de:443/website/VirtualHostRoot/website/autoren/der-freitag/RANDOM-ARTICLE.

But that's quite inconvenient. Probably VHM sets enough VIRTUAL_-prefixed request variables that we could prefix queued tasks with that automatically.

datakurre commented 8 years ago

PS. While digging into this, I realized, that you can actually quite easily capture tasks requests in tests by simply setting

    self.layer['server'].handler = ... 

to your test handler accepting args app, request, response. Just store the default handler by fixture in test setup and return it back in teardown, and you may even call the default handler in test to get the default behavior. If you don't call the default handler, the handler should at least

     response.stdout.write('HTTP/1.1 204\r\n')
     response.stdout.close()

to please the task queuing/consuming server.