Rasa: DucklingHTTPExtractor does not define a request timeout, possibly leading to pipeline hanging

[efung]

Rasa NLU version: 0.13.1

Operating system (windows, osx, ...): macOS 10.14.3

Issue:

In a pipeline that includes ner_duckling_http, if the server takes a long time to respond, the pipeline will block waiting for a response, potentially leading to an application hanging indefinitely. The issue is that the call to requests.post() does not specify any timeout. The Python Requests documentation says:

You can tell Requests to stop waiting for a response after a given number of seconds with the timeout parameter. Nearly all production code should use this parameter in nearly all requests. Failure to do so can cause your program to hang indefinitely

If no timeout is specified explicitly, requests do not time out.

(Emphasis mine)

One way to demonstrate this is to configure a simple, local server that doesn't respond (but accepts TCP connections), e.g. using netcat:

$ nc -l 8000 -k

Then, configure the Rasa pipeline with a DucklingHTTPExtractor with the url pointing to localhost:8000. Then, run a parse query. You should see that the netcat server gets a request:

HTTP:/1 200 OK
POST /parse HTTP/1.1
Host: localhost:8000
User-Agent: python-requests/2.19.1
Accept-Encoding: gzip, deflate
Accept: */*
Connection: keep-alive
Content-Type: application/x-www-form-urlencoded; charset=UTF-8
Content-Length: 72

text=hello&locale=en_US&reftime=1549498165000^C

However, the request to the Rasa NLU server will hang, and not return.

[efung]

Would it make sense for a default timeout to be added to EndpointConfig, and then the Duckling HTTP class be refactored to use that?

[akelad]

Thanks for raising this issue, @EPedrotti will get back to you about it soon.

[EPedrotti]

hi @efung would you mind creating a PR for this?

[netcarver]

Does this impact other uses of requests.post() in rasa_core too? Grep'ing shows four instances in that code base...

rasa_core/rasa_core/channels/botframework.py:53:
rasa_core/rasa_core/channels/botframework.py:91:
rasa_core/rasa_core/channels/console.py:55:
rasa_core/rasa_core/channels/console.py:71:

[EPedrotti]

@netcarver that is correct. Would you like to create a PR for core as well?

@efung @netcarver good catch guys!

[efung]

I can't get to this in the short term, but will see if I have time later.
But I agree that centralizing the HTTP client code is a good approach.

On Thu, Feb 14, 2019, 07:24 Edoardo Pedrotti <[email protected]
wrote:

@netcarver https://github.com/netcarver that is correct. Would you like
to create a PR for core as well?

@efung https://github.com/efung @netcarver
https://github.com/netcarver good catch guys!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/RasaHQ/rasa_nlu/issues/1695#issuecomment-463608422,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AASYJssJGcLvLxZVG6MoqjtbTjf_9Oiqks5vNVWPgaJpZM4amY8G
.

[akelad]

@efung are you still interested in creating a PR for this?

[efung]

@efung are you still interested in creating a PR for this?

Sorry, I have no time to do this at the moment.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[vishnuvrpriya]

Hi @akelad ,
I have fixed this bug through the PR: #4507
I request you to review the PR and let me know if you have any feedback and help in merging the changes, if they look good.
This is my first contribution so I would be glad to receive any feedback on my work 😃

Thanks,
Vishnu Priya VR.

[vishnupriyavr]

Hi @akelad ,
This is fixed and merged by @imLew under PR #4535 .Can you please push this LIVE and mark it closed?Thanks!

[akelad]

I'm afraid it's been merged to master and so would be part of the next major release. @imLew any reason it wasn't merged to 1.3.x?

[imLew]

@vishnupriyavr it has just been merged into 1.3.x and will be out with the next patch