Can not get observable from phone, but can from browser

Hi,

Currently I have an app that was made using ionic 3, which was updated over a year ago.
Today we wanted to push a new update to the app.

We’ve updated some angular stuff and now there are loads of deprecated functions that we have replaced.

In the browser when I run ionic serve. All of the information is being displayed from the observables that are returned.

However in android, this information doesn’t want to display in the UI.
I’ve setup a LOT of debug messages to see what is being shown.

The following function should return an observable that can be subscribed to and then displays data.
A console log inside the subscribe of this function (other page) doesn’t show up.

The following console logs inside of this function are like followed:
res: shows all of the responses correctly.
response: shows the current blob of an image correctly.
nieuws1: doesn’t show up.
finish1: shows a observable.
nieuws2: doesn’t show up.
finish2: doesn’t show up.
nieuws3: shows an empty variable with 0 length.
Observables: shows all of the observables corresponding to res (same amount).
“Gets Here”: doesn’t show up.
nieuws4: doesn’t show up.
result: shows an observable.

getNieuwsPlaats(plaats) {
    return observableFrom(this.connection.magDataOphalen().then((waarde) => {
      let body = new FormData();
        body.append('actie', 'nieuws');
        body.append('plaats', plaats);
      if (waarde) {
        return this.http.post(Globals.api, body).pipe(mergeMap((res:any[]) => {
          console.log(res);
          let nieuws:Nieuwsbericht[] = new Array();
          let Observables:Observable<any>[] = new Array();
          let finish:Observable<any>;
          if (res.length != 0) {
            for (var i = 0; i < res.length; i++) {
              if (res[i].Prioriteit == "0") {
                res[i].Prioriteit = false;
              }
              else {
                res[i].Prioriteit = true;
              }
              let temp = res[i];
              if (res[i].Afbeelding) {
                Observables.push(this.storage.getImage(Globals.host + res[i].Afbeelding).pipe(mergeMap( response =>{
                  console.log(response);
                  var blob = response;
                  var reader = new FileReader();
                  reader.readAsDataURL(blob);
                  let file;
                  finish = new Observable((observer) => {
                    reader.onloadend = () => {
                      file = reader.result;
                      nieuws.push(new Nieuwsbericht(temp.Datum, temp.Onderwerp, temp.Bericht, temp.Afdeling, file, temp.Prioriteit));
                      console.log(nieuws1);
                      observer.next(nieuws);
                      observer.complete();
                      }
                  });
                  console.log(finish1);
                  return finish;
                })));
              }
              else {
                finish = new Observable((observer) => {
                  nieuws.push(new Nieuwsbericht(temp.Datum, temp.Onderwerp, temp.Bericht, temp.Afdeling, '', temp.Prioriteit));
                  console.log(nieuws2);
                  observer.next(nieuws);
                  observer.complete();
                });
                console.log(finish2);
                Observables.push(finish);
              }

            }
            console.log(nieuws3);
            console.log(Observables);
            return observableForkJoin(Observables).pipe(map(() => {
              console.log("Gets here");
              nieuws.sort(function(a, b){
                let d1 = moment(a.datum);
                let d2 = moment(b.datum);
                if (d1 > d2) return -1;
                if (d1 < d2) return 1;
                return 0;
              });
              
              nieuws.forEach(function(bericht) {
                let newDatum = moment(bericht.datum);
                bericht.datum = newDatum.format('DD-MM-YYYY HH:mm').toString();
              });
              this.storage.isReady().then(() => {
                this.storage.storeItem('nieuws-' + plaats, nieuws);
              });
              console.log(nieuws4);
              return nieuws;
            }));
          }
          else {
            let result:Observable<any>;
            result = new Observable((observer) => {
              observer.next("Er is geen data aanwezig.");
              observer.complete();
            });
            return result;
          }
      }));    
      }
      else {
        let result:Observable<any>;
        result = new Observable((observer) => {
          this.storage.isReady().then(() => {
            let data = this.storage.getItem('nieuws-' + plaats);
            data.then(value => {
              observer.next(value);
              observer.complete();
            });
          });
        });
        return result;
      }
    })).pipe(mergeMap((result) => {
      console.log(result);
      return result;  
    }));
  }

I have this problem with all of my information inside of my app (different functions), and apparently this only happens with information that has images to download.

When I look in the network tab, I’m getting 200 OK’s but I’m not seeing the blobs coming up.

All of the console logs do show up in the browser when I use ionic serve.

What is this function? I’m guessing it returns a Promise, but is it your Promise? Does it come from a third-party library? If so, which one?

It comes from a service I have setup (not a third party library) that decides whether the user is allowed to retrieve data.

What it does is check if the user is on wifi, if it is, it is allowed to retrieve data. When it’s on a cellular network, it will popup a notice asking the user whether they want to use data from a cellular network.

They can then agree or decline and when it’s declined the user is not allowed to retrieve data and the app will only show offline stored data.

This promise comes from my own service, and tends to work in the previous version of our app with roughly the same codebase.

I haven’t checked whether the issue resides there, but I doubt it since the console logs within them are displayed.

Not sure if this could be the problem. I’ll have a look tomorrow.

I can’t display the full source code.

The this.storage.getImage function basically retrieves an image through an http request that returns a blob.

Totally irrevelant to my suspicion, which is that you’re relying on a Promise that comes from some external source that is not Angular-aware. When this happens, you jump out of Angular’s zones and run into all sorts of trouble.

The least intrusive way to test that theory would be to wrap every Promise that comes from an external source in Promise.resolve: Promise.resolve(functionReturningSketchyPromise()).

Incidentally, I at least find getNieuwsPlaats incredibly hard to read and follow. Not that you need care about my opinion, but if this were sent to me for code review, I would recommend the following:

  • get serious about naming conventions. camelCase for variables and properties, PascalCase for types
  • get out of Promise-land and into Observable-land ASAP - convert each Promise immediately to an Observable and work with it
  • no nesting of futures, only chaining - the whole thing should read like an assembly line:
getNieuwsPlaats(plaats: WhateverTypeThisIs): Observable<WhateverItIsReturning[]> {
  return from(this.connection.magDataOphalen()).pipe(
    map(mdo => ...),
    mergeMap(foo => ...),
    ...
  );
}
  • try to follow functional programming conventions as best you can; reduce external state modification
  • try to eliminate all manual Observable creation - it’s verbose and virtually always unneeded
  • try to eliminate every single any in your code - I would be surprised if any of them were needed in this function at least

You are absolutely right.

This code was written 3 years ago when I first started learning angular and I used some pretty bad practices that I now know should’ve been written differently.

I’m having a hard time digging through this old project because with my knowledge back then it was rather poorly written.

I’ll have a look at converting the promise into an observable and I’ll also clean up my code.

The reason I created the observables was to forkjoin them during the function call so that I was certain that all of my images were downloaded before returning the other observable.

But this should be handled differently anyhow.
I’d actually rather have my images seperate from my text so that I can load them dynamically while already displaying text instead of waiting for all the downloads.

Thanks for the guidelines on what should happen next.

Tomorrow I will rewrite this part of the code (not all of the code since the project is huge and has hundreds of other functions like these…) I will have to fix a lot of them… >.<

If I find out it is indeed the code making calls outside the angular zone. I’ll try to solve this.

I’m still a bit flabbergasted why this would only occur on android though. And not the browser.

First thing I’m going to check is what happens if I completely eliminate the magdataophalen check.

Does this somewhat look better? Or can it still be improved?

  getNieuwsPlaats(plaats):Observable<NieuwsBericht[] | string> {
    return this.connection.magDataOphalen().pipe(mergeMap((waarde: boolean) => {
      let body = new FormData();
      body.append('actie', 'nieuws');
      body.append('plaats', plaats);
      if (waarde) {
        return this.http.post(Globals.api, body).pipe(map((res: NieuwsBericht[]) => {
          let nieuws:NieuwsBericht[] = new Array();
          if (res.length != 0) {
            res.forEach((temp: NieuwsBericht) => {
              if (temp.afbeelding) {
                this.storage.getImage(Globals.host + temp.afbeelding).subscribe( response =>{
                  var blob = response;
                  var reader = new FileReader();
                  reader.readAsDataURL(blob);
                  let file;
                  reader.onloadend = () => {
                    file = reader.result;
                    temp.afbeelding = file;
                    nieuws.push(temp);
                  }
                });
              }
              else {
                temp.afbeelding = '';
                nieuws.push(temp);
              }
            });
            nieuws.sort(function(a, b){
              let d1 = moment(a.datum);
              let d2 = moment(b.datum);
              if (d1 > d2) return -1;
              if (d1 < d2) return 1;
              return 0;
            });
              
            nieuws.forEach((bericht: NieuwsBericht) => {
              let newDatum = moment(bericht.datum);
              bericht.datum = newDatum.format('DD-MM-YYYY HH:mm').toString();
            });

            this.storage.isReady().then(() => {
              this.storage.storeItem('nieuws-' + plaats, nieuws);
            });

            return nieuws;
          }
          else {
            return "Er is geen data aanwezig.";
          }
        }));
      }
      else {
          this.storage.isReady().then(() => {
            let data = this.storage.getItem('nieuws-' + plaats);
            data.then(value => {
              return value;
            });
          });
      }
    }));
    
  }

This seems to work on browser, I’m about to test it on mobile.
I will have to rewrite my entire data service…
In order to even test this I had to almost comment out all of my pages except the ones fetching this data.

Apparently I’m now getting an empty nieuws variable returned.

Possibly because it’s returning nieuws before the images have been downloaded. (see the subscribe for storage.getImage).

So now instead of just not doing anything, it simply displays that it doesn’t have anything.
It’s a step forward but not yet what I want to accomplish completely.

I think I didn’t do the chaining correctly, I’m still nesting futures…

Yes on both counts. As you noted, it’s still nesting, but at least now I can see what I think is the fundamentally problematic nesting: the image reading loop.

I assume you’ve had some experience with imperative programming, because that’s what this style looks like to me (I wrote a lot of code like this when I was starting web apps): do this, then do that, then do this.

You simply can’t write webapps this way. You have to completely let go of the illusion that code that physically sits on line 342 is going to execute before code that is on line 349.

Which means that here’s what you think your function looks like:

listOfStories = getListOfStories();
photos = new Array();
for (each story of listOfStories) {
  readAPhotoAndAddItToAnArray(story.photo, photos);
}
return photos;

Sadly, readAPhotoAndAddItToAnArray doesn’t guarantee when it will get around to adding to that array, so you have absolutely zero idea how many of them will be on photos at the time you return it. In reality, zero is the most likely number. Incidentally, it’s really easy to write mock test suites where asynchronous stuff does behave synchronously, lulling us into a false sense of security.

This is why a functional (as opposed to imperative) programming mindset is so crucial here. In functional programming, you have to get extremely myopic: no messing around with anything outside our function - all functions have to explicitly take in everything they need and return everything needed from them. It seems like a masochistic set of restrictions, but following it completely eliminates huge classes of subtle bugs like this.

I get the feeling that you are more or less facing the same situation as this thread, and hopefully the discussion therein will be of some use to you.

3 Likes

Thank you rapropos.

This explains a lot to me. I am aware that code that is physically sitting on a higher line count can be executed before other code. I’ve experienced these sort of things in the past. And I’m pretty confident that the images are not altered (added) before the information has already returned to the subscriber.

Since I am now effectively seeing the information display without images…

And I’m having a hard time figuring out how I can do all of this in a functional programming way. I’ll have a look at the topic you’ve linked me. I find it very fascinating.

I’ve decided to split the function into 2 seperate functions. One for fetching all the information of the interface. And one for downloading the images and adding it to each NieuwsBericht.

Not only will this make my code shorter and easier to read. I will also have seperate logic for my images so that I can load my information seperate from my images (make some sort of animation while they load and then creating a reveal effect when they’re finished downloading for example.)

This should also make it seem as a faster interface. Since you are presented with information instead of waiting on all images to download before displaying something.

I’ll have to rewrite almost all of my api function calls though. So it might take some time. But it’s mostly repetative work.

I’ll be working on this tomorrow.

Thanks for all the helpful information!

Edit: looking at your code examples in the other thread. I think I know exactly how I’m going to tackle this.

Tomorrow I’ll create another reply to show the final code as an example in this thread.

1 Like

By moving my can retrieve data functionality somewhere else, I now have succesfuly changed it into something like this:

   getNewsPerPlace(place: string):Observable<NewsMessage[] | NewsMessage | string> {
    let body = new FormData();
    body.append('action', 'news');
    body.append('place', place);
    return this.http.post(Globals.api, body).pipe(map((response: NewsMessage[] | NewsMessage) => {
      if (response instanceof Array) {
        response.sort(function(a, b){
          let d1 = moment(a.date);
          let d2 = moment(b.date);
          if (d1 > d2) return -1;
          if (d1 < d2) return 1;
          return 0;
        });
        response.forEach((message) => {
          let newDate = moment(message.date);
          message.date = newDate.format('DD-MM-YYYY HH:mm').toString();
        });
        return response;
      } 
      else if (response) {
        return response;
      } 
      else {
        return "There is no data available";
      }
    })); 
  }

Basically I now also have a separate function for downloading the images.
This one will display the images from a url, and afterwards has a store function to store all of the data locally.

1 Like

One more thing: ditching moment for date-fns will, in addition to likely reducing your overall app bundle size, allow you to simplify your date wrangling:

// lodash's functions are more robust than directly checking instanceof
if (isArray(response)) {
  response.sort((a, b) => compareAsc(a.date, b.date));
  // I would leave each message.date as a Date object here,
  // and do the formatting in the presentation layer
}

Whereever you choose to do the stringifying of the dates, you can also save one more line and moment allocation: format(message.date, "dd-MM-yyyy HH:mm")

Thank you <3 will do.