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.

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

REGRESSION in 3.1.0: http download gets stuck after headers, as if not getting data #9035

Open
6 tasks done
php4fan opened this issue Nov 19, 2023 · 6 comments
Open
6 tasks done

Comments

@php4fan
Copy link

php4fan commented Nov 19, 2023

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Platform

  • Hardware: Sparkfun ESP8266 Thing Dev
  • Core Version: I don't know, downloaded today
  • Development Env: [Arduino IDE|]
  • Operating System: [Manjaro Linux]

Settings in IDE

  • Module: [Sparkfun ESP8266 Thing Dev]
  • Flash Mode: from the Arduino IDE
  • Flash Size: no idea
  • lwip Variant: no idea
  • Reset Method: no idea
  • Flash Frequency: [40Mhz]
  • CPU Frequency: [80Mhz|160MHz]
  • Upload Using: [SERIAL]
  • Upload Speed: [115200] (serial upload only)

Problem Description

In 2019 I wrote a sketch that downloads text from a url using HTTPClient. I uploaded it to a few boards and it has been working flawlessly, and still is.

Now I updated Arduino and the ESP8266 core to the latest version, re-compiled my code (I needed to change a call to HTTPClient::begin() because of BC-breaking changes in the API) and uploaded it to one of the boards), and now the download systematically gets stuck after downloading the http headers, waiting forever for the next bytes to become available.

The hardware is the same, the network is the same, the server I'm downloading from is the same, my code hasn't changed one bit (except again for the HTTPClient::begin() call). Literally the only thing that has changed is I switched to the latest version of Arduino IDE, the core and the libraries.

Code that used to work (that was well tested and has been working without issue for 4 years on a dozen of devices on different wifi networks) is now broken after the update.

Unfortunately I hadn't updated the libraries in the last 4 years, so I don't know when in the last 4 years the bug appeared.

I'm sorry I don't have the time to create a minimal but full working example, and I cannot share the full code because it's too complex. I'll share the part of the code that does the http download.

MCVE Sketch

  uint8_t httpbuf[128] = {0};

  serial.printf("... Loading from url: %s\n", url.c_str());
  int httpcode=200;
  int len=KNOWN_LENGTH;
  int bytesdownloaded=0;

  http.begin(wificlient, url);
  httpcode=http.GET();
  if (httpcode==200) {
    len=http.getSize();
  }
  
  serial.printf("... got HTTP code %d; size is %d\n", httpcode, len);
  int ticks=0;
  int ticks0bytes=0;

  if (httpcode==304) {
    serial.printf("No new file to download (HTTP 304 response)\n");
    return 0;
  }

  if (httpcode==200) {
    if (len==-1) {
      serial.printf("Assuming size %d\n", KNOWN_LENGTH);
      len = KNOWN_LENGTH;
    }
    ledCode(LEDCODE_HTTPGET_OK); // this is just a function that blinks the led with one of a set of given patterns


    int timestarted = millis();
    int timechecked = millis();
    int timegotbytes = millis();

    while (len > 0 || len ==-1) {
      if (millis()-timestarted > DOWNLOAD_MAX_TIME*1000) {
        serial.printf("Image download is taking too long. Aborting.\n");
        return LIERR_TIMEOUT;
      }
      int gotbytes=0;

      if (!http.connected()) {
          break;
      }
      size_t avl=wificlient.available();
      if (avl) {
          gotbytes = wificlient.readBytes(httpbuf, ((avl>sizeof(httpbuf))?sizeof(httpbuf):avl));
          bytesdownloaded+=gotbytes;
          ticks0bytes=0;
      }
      else {
          delay(100);
          ticks0bytes++;
          if (ticks0bytes>=20) {
            serial.printf("... !! been waiting for available bytes for more than 2 seconds!!\n");
            ticks0bytes=0;
            ticks=10000;
          }
      }

      if (gotbytes>0) {
        if (len>0) len=max(0, len-gotbytes);
        ticks+=gotbytes;
        if (millis()-timechecked > 5000) {
          timechecked=millis();
          ticks+=10000;
        }
        if (ticks>=10000) {
          ledCode(LEDCODE_HTTPSTREAM_GOTBYTES); // blink once
          serial.printf("...((ticks = %d)) %d bytes downloaded; %d remaining\n", ticks, bytesdownloaded, len);
          ticks=0;
        }

        for (int j=0; j<gotbytes; j++) {
          //Do stuff with the data downloaded so far
        }

        timegotbytes = millis();
      }
      else {
        if (millis()-timegotbytes > DOWNLOAD_MAX_TIME_NODATA*1000) {
          serial.printf("... !! No data for too long. Aborting.\n");
          return LIERR_TIMEOUT;
        }
      }

    }
    serial.printf("Done downloading file (%d bytes downloaded).\n", bytesdownloaded);
    ledCode(LEDCODE_IMAGE_DONE);

    return 0;
  }
  else {
    serial.printf("...HTTP Error %d\n", httpcode);
    return LIERR_HTTP_ERROR;
  }

Debug Messages

The output from the code above is:

... Loading from url: ******************************************
... got HTTP code 200; size is 384000
... 0 bytes available.
... 0 bytes available.
... 0 bytes available.
... 0 bytes available.
... 0 bytes available.
... 0 bytes available.
... 0 bytes available.
... 0 bytes available.
... !! been waiting for available bytes for more than 2 seconds!!

  [.... many times...]

... 0 bytes available.
... 0 bytes available.
... 0 bytes available.
... 0 bytes available.
... 0 bytes available.
... 0 bytes available.
... !! No data for too long. Aborting.
@php4fan
Copy link
Author

php4fan commented Nov 20, 2023

I tested on several versions. It broke somewhere between:

3.0.2 Works
3.1.0 Broken

@php4fan php4fan changed the title REGRESSION: http client broken by some update in the last 4 years REGRESSION in 3.1.0: http download gets stuck after headers, as if not getting data Nov 20, 2023
@d-a-v
Copy link
Collaborator

d-a-v commented Nov 20, 2023

This might be #8237, can you try and change wificlient to http.getStream():

      size_t avl = wificlient.available();
          gotbytes = wificlient.readBytes(httpbuf, ((avl>sizeof(httpbuf))?sizeof(httpbuf):avl));

to

      size_t avl = http.getStream().available();
          gotbytes = http.getStream().readBytes(httpbuf, ((avl>sizeof(httpbuf))?sizeof(httpbuf):avl));

@mcspr
Copy link
Collaborator

mcspr commented Nov 20, 2023

This might be #8237, can you try and change wificlient to http.getStream():

Right, clone() copies client ctx but does not share it. Have to remove its self-deletion first (see delete this;), to persist it across objects. iirc I gave up writing adaptor b/c we have to re-write client & secure-client anyway (...at some point :)

@php4fan
Copy link
Author

php4fan commented Nov 21, 2023

Indeed, the workaround works.

Also this works:

//...
httpcode=http.GET();
WiFiClient httpstream = http.getStream(); // <<<<<<<<<<
//...
//....
      size_t avl = httpstream.available();
      gotbytes = httpstream.readBytes(// .....

but only as long as getStream() is called after http.GET().

I had actually tried some variation of this (without knowing why, but suspecting that getStream "did something weird"), but crucially I had put the getStream() call before http.GET() and when it made no difference I was like "of course it doesn't, what a crazy idea 🙄".

@d-a-v
Copy link
Collaborator

d-a-v commented Nov 22, 2023

You can also try with this

toSend = min(1000, stillToSend); // 1000 depending on how much time you can spend in Stream::sendSize()
stillToSend -= http.getStream().sendSize(file, toSend); // file is a Stream

@oarcher
Copy link

oarcher commented Apr 8, 2024

I've tried the workaround with getStream(), and it works for me too.

But it's 100x time slower to download!!

The workaround is backward compatible, but it's still fast on 3.0.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants