Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Infinite loop (or almost) with OctoprintApi::sendRequestToOctoprint() #28

Open
fmatray opened this issue Jun 1, 2020 · 4 comments
Open

Infinite loop (or almost) with OctoprintApi::sendRequestToOctoprint() #28

fmatray opened this issue Jun 1, 2020 · 4 comments

Comments

@fmatray
Copy link
Contributor

fmatray commented Jun 1, 2020

Hello,
The the method OctoprintApi::sendRequestToOctoprint() seems, with some occasion, loop for ever or almost.

One of the occasion is with this answer (octoprint is down and Nginx is responding) :

HTTP/1.0 502 Bad Gateway
Cache-Control: no-cache
Connection: close
Content-Type: text/html

<html><body><h1>502 Bad Gateway</h1>
The server returned an invalid or incomplete response.
</body></html>

The code is waiting for a "Content-Length" that never happen.

What do you think about using ArduinoHttpClient instead of a custom code ?
https://github.com/arduino-libraries/ArduinoHttpClient
@chunkysteveo
Copy link
Owner

I could take a look at the code to see if we can catch this event. The custom code was to allow for any network client (esp8266, esp32, ethernet etc) to connect without dependencies.

We could utilise the return code (502) if not 200, and break out the loop? Will take a look on a break - but good spot - you're really pushing the library to some good edge cases! :)

steve

@fmatray
Copy link
Contributor Author

fmatray commented Jun 2, 2020

I’m not sure this is the only event with such problem . And it’s not easy to test all cases: direct request to octoprint, Nginx, Apache, Haproxy, etc. It could depend on the configuration.

Custom codes for generic function, like http request, tend to be more buggy, as they are not tested as well as a standard library. That’s why, high level library focused should rely on mid level ones.

A way to keep the compatibility with Arduino and ESPxxxx is to use directives:

#if defined(ARDUINO)
// Arduino specific code 
#elif defined(ESP8266)
// ESP8266 specific code
#else
#Error “Platerform not supported”
#endif

I’m haven’t the exact code, it’s just for the idea. I have no more ESPxxxx board at home, I can only test with an MKR 1010.

fmatray added a commit to fmatray/OctoPrintAPI that referenced this issue Jun 2, 2020
@fmatray
sendRequestToOctoprint():
920890c 
- Resole issue chunkysteveo#28
- Reduce memory footprint with less variables
- Split header sending in sendHeader()
- Timeout run through all request
- Refactor main loop for a better readability

WARNING: Only tested with GET (Octoprint up and down with Haproxy, with printer up and down).
	 Not tested yet with POST commands.

Reduce variable memory for octoPrintPrintHeadRelativeJog() and remove a potential buffer overflow.

Remove unsued maxMessageLength and httpErrorBody
@fmatray fmatray mentioned this issue Jun 2, 2020
Open
@fmatray
Copy link
Contributor Author

fmatray commented Jun 6, 2020

After 4 days of test, I have no more infinite loops.

The problem was with empty responses like 502 and, also, with the Wifi driver.

On the MKR 1010, perhaps with other boards, if WifiClient::available() is called too often, the driver receives errors from the ESP and can't deal with it. That's why there is delay(100) while waiting.

@chunkysteveo
Copy link
Owner

you've been busy again! Let me check out the updates and have a play with my ESP boards.

Steve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@fmatray @chunkysteveo