0

I'm having some trouble to figure out why my await statement is not respected and code proceeds before the completion the async function.

My code:

  async getSensors(boardId: number): Promise<Sensor[]> {
    console.log('call sensor');
    const sensors: Sensor[] = [];
    await this.http.get<object[]>(API_URL + 'sensors/' + boardId).toPromise().then(res => {
      console.log('http sensor');
      res.forEach(r => sensors.push(GenericDeserialize(r, Sensor)));
    });
    console.log('return sensor');
    return sensors;
  }

  async getRelays(boardId: number): Promise<Relay[]> {
    console.log('call relay');
    const relays: Relay[] = [];
    await this.http.get<object[]>(API_URL + 'relays/' + boardId).toPromise().then(res => {
      console.log('http relay');
      res.forEach(r => relays.push(GenericDeserialize(r, Relay)));
    });
    console.log('return relay');
    return relays;
  }

  async getBoards(): Promise<Board[]> {
    const boards: Board[] = [];
    await this.http.get<object[]>(API_URL + 'boards').toPromise().then(async res => {
      console.log('http boards');
      await res.forEach(async b => {
        console.log('loop', b);
        const board = GenericDeserialize(b, Board);
        await this.getSensors(board.boardId).then(sensors => board.sensors = sensors);
        await this.getRelays(board.boardId).then(relays => board.relays = relays);
        boards.push(board);
      });
    });
    console.log('return boards');
    return boards;
  }

My jasmine test:

  it('should have two sensors and one relay', async () => {
    const service: DataService = TestBed.get(DataService);
    const boards = await service.getBoards();
    const board = boards[0];
    expect(board.sensors.length).toBe(2);
    expect(board.relays.length).toBe(1);

  });

This is the actual "callstack" that the console outputs when I run my test:

'return relay'
'http relay'
'return relay'
'http boards'
'loop', Object{board_id: 0, board_name: 'test'}
'call sensor'
'loop', Object{board_id: 1234, board_name: 'esp1'}
'call sensor'
'return boards'

And this is the "callstack" that I was expecting:

'http boards'
'loop', Object{board_id: 0, board_name: 'test'}
'call sensor'
'http sensor'
'return sensor'
'call relay'
'http relay'
'return relay'
'loop', Object{board_id: 1234, board_name: 'esp1'}
'call sensor'
'http sensor'
'return sensor'
'call relay'
'http relay'
'return relay'
'return boards'
fadlio
  • 5
  • 3
  • you either have to use `async/await` **or** `.then()` – messerbill Jun 28 '19 at 13:38
  • @messerbill - That's not actually true. But it's generally best to use only one or the other. – T.J. Crowder Jun 28 '19 at 13:39
  • I was desperate, so I used both of them, just to be sure, haha. I will fix that too, thank you! – fadlio Jun 28 '19 at 13:55
  • @T.J.Crowder how will the script behave if I make use of both? Will both (`await` **and** `.then()`) be executed? – messerbill Jun 28 '19 at 14:32
  • 1
    @messerbill - Yes. If you have `x = await somePromise.then(y => y * 2)`, you're awaiting the promise returned by `then` (which is resolved to `somePromise`), not `somePromise`. So when `somePromise` is fulfilled, the `then` handler is called, `then`'s promise is fulfilled with the result, and the `await` expression is fulfilled. In that example, if `somePromise` fulfills with 21, `x` receives 42 (because `y => y * 2` returns 42). – T.J. Crowder Jun 28 '19 at 14:53
  • @T.J.Crowder thanks for this explanation – messerbill Jun 28 '19 at 15:04

1 Answers1

4

Your problem is here:

await res.forEach(async b => {
  console.log('loop', b);
  const board = GenericDeserialize(b, Board);
  await this.getSensors(board.boardId).then(sensors => board.sensors = sensors);
  await this.getRelays(board.boardId).then(relays => board.relays = relays);
  boards.push(board);
});

forEach always returns undefined, and await undefined only waits one microtick; and forEach completely ignores the return value of its callback, so the promises the callbacks create are never used for anything, and nothing waits for them.

If you want to process those in parallel, you can just change it to map instead and add Promise.all:

await Promise.all(res.map(async b => {
  console.log('loop', b);
  const board = GenericDeserialize(b, Board);
  await this.getSensors(board.boardId).then(sensors => board.sensors = sensors);
  await this.getRelays(board.boardId).then(relays => board.relays = relays);
  boards.push(board);
}));

If you want to do them in series, use a loop:

for (const b of res) {
  console.log('loop', b);
  const board = GenericDeserialize(b, Board);
  await this.getSensors(board.boardId).then(sensors => board.sensors = sensors);
  await this.getRelays(board.boardId).then(relays => board.relays = relays);
  boards.push(board);
}

Side note: You're doing this.http.get<object[]>(API_URL + /*...*/).toPromise() a lot. I'd tend to put that in a utility method:

callAPI(call: string): Promise<object[]> {
    return this.http.get<object[]>(API_URL + call).toPromise();
}

That also makes it reasonable to change something like this:

await this.http.get<object[]>(API_URL + 'relays/' + boardId).toPromise().then(res => {
  console.log('http relay');
  res.forEach(r => relays.push(GenericDeserialize(r, Relay)));
});

to

relays.push(...(await this.callAPI('relays/' + boardId)).map(r => GenericDeserialize(r, Relay)));

or if you really want that console.log:

relays.push(...(await this.callAPI('relays/' + boardId)).map(r => {
  console.log('http relay');
    return GenericDeserialize(r, Relay);
}));
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875