Javascript Promise Anti-Pattern error

Question

I am using Ionic 2 with SQLite. I am getting an error, and I suspect it is due to me not using Promises correctly.

I get the following error:

ERROR REFRESHING CHATS: {}

TypeError {stack: (...), message: "Cannot read property 'executeSql' of undefined"}

ts

private database = new SQLite();

public openDatabase(): Promise<Array<Message>> {
    let promise: Promise<Array<Message>> = new Promise<Array<Message>>(resolve => {
        if (this.database) {
            let promiseChats: Promise<any> = this.refreshChats();
            let promiseMessages: Promise<any> = this.refreshMessages();
            Promise.all([promiseChats, promiseMessages]).then(() => { resolve(this.messages) });
        } else {
            this.database.openDatabase({
                name: "data.db",
                location: "default"
            }).then(() => {
                let promiseChats: Promise<any> = this.refreshChats();
                let promiseMessages: Promise<any> = this.refreshMessages();
                Promise.all([promiseChats, promiseMessages]).then(() => { resolve(this.messages) });
            }, (error) => {
                console.log("OPEN ERROR: ", error);
            });
        }
    });
    return promise;
}

public refreshChats(): Promise<any> {
    return this.database.executeSql("SELECT * FROM chats", [])
        .then((chatData) => {
            this.chats = [];
            if (chatData.rows.length > 0) {
                for (var i = 0; i < chatData.rows.length; i++) {
                    this.populateChat(chatData, i);
                }
            }
            return this.chats;
        })
        .catch(error => {
            console.log("ERROR REFRESHING CHATS: " + JSON.stringify(error));
            console.log(error);
        });
}

When I debug the code, and break on this.database.executeSql, this.database is not undefined. Also, you can see there is a check on this.database to see if it's undefined before too. Yet, the debugger steps out the Promise into the error, reporting the error shown.

Also, after the Promise finishes, the this.chats are populated. So I am very confused.

If anyone can suggest what I am doing incorrectly, I would appreciate it.

UPDATE

I have updated the code as follows, but still get the same error:

public openDatabase(): Promise<Array<Message>> {
    let promise: Promise<Array<Message>> = new Promise<Array<Message>>(resolve => {
        if (this.database) {
            Promise.all([this.refreshChats(), this.refreshMessages()]).then(() => { resolve(this.messages) });
        } else {
            this.database.openDatabase({
                name: "data.db",
                location: "default"
            }).then(() => {
                Promise.all([this.refreshChats(), this.refreshMessages()]).then(() => { resolve(this.messages) });
            }, (error) => {
                console.log("OPEN ERROR: ", error);
            });
        }
    });
    return promise;
}

public refreshChats(): Promise<any> {
    return this.database.executeSql("SELECT * FROM chats", [])
        .then((chatData) => {
            let promises: Array<any> = [];
            this.chats = [];
            if (chatData.rows.length > 0) {
                for (var i = 0; i < chatData.rows.length; i++) {
                    promises.push(this.populateChat(chatData.rows.item(i)));
                }
            }
            Promise.all(promises).then(() => {
                return this.chats;
            });
        })
        .catch(error => {
            console.log("ERROR REFRESHING CHATS: " + JSON.stringify(error));
            console.log(error);
        });
}

1. It's possible that `this` in `populateChat()` is not the `this` you expect it to be. — Roamer-1888, Oct 17 '16 at 10:29
2. Code is confusing. `refreshChats()` loops through `chatData.rows` but in the loop calls `this.populateChat(chatData, i)`, which iteslf works on `chatData.rows.item(i)`. It would make more sense to call `this.populateChat(chatData.rows.item(i))`. — Roamer-1888, Oct 17 '16 at 10:29
3. You know that `this.database.executeSql()` is asynchronous because you have `this.database.executeSql(...).then()` in two places. But (a) `populateChat()` doesn't return a promise (as it should), and `refreshChats ()` treats `populateChat` as if it were synchronous (it needs to agggregate returned promises). — Roamer-1888, Oct 17 '16 at 10:29
Please don't keep changing the question - comments (like mine) and answers no longer relate to what is now being asked. By all means add, but don't overwrite. — Roamer-1888, Oct 17 '16 at 10:36
@Roamer-1888, apologies for updating the code. I understand that. — Richard, Oct 17 '16 at 10:45
Your code is not safe: you are not checking `this.database` after resolving the `openDatabase` promise. Your assumption that it is _never_ `undefined` is clearly wrong. The fact that your array is populated regardless suggests that the condition occurs at the end of the sequence. Did you set a conditional breakpoint on that line? — Cool Blue, Oct 19 '16 at 15:54

score 0 · Answer 1 · edited May 23 '17 at 12:33

You are employing a bit of a Promise antipattern which I will describe to you. Although I can't see a reason it would cause your problem, it may help with debugging.

It's known as the explicit-construction anti-pattern and essentially it is when you create a new Promise unnecessarily.

For instance in your code, both of your functions create a new Promise at the start, where they could instead just return the promise used later;

public refreshChats(): Promise<Array<Chat>> {
  if (!this.database || this.database === null) {
    console.log('ERROR refreshing chats: database = ' + this.database);
  }

  return this.database.executeSql("SELECT * FROM chats", [])
    .then((chatData) => {
      this.chats = [];
      if (chatData.rows.length > 0) {
        for (var i = 0; i < chatData.rows.length; i++) {
           this.populateChat(chatData, i);
        }
      }
      return this.chats;
    })
    .catch(error => {
      console.log("ERROR REFRESHING CHATS: " + JSON.stringify(error));
      console.log(error);
    });
}

This may help to break up your error so that you can see where it originates. If you produce any further information, ping me and I will update my answer.

Thanks, that does simplify the code too. I will give it a try. — Richard, Oct 17 '16 at 09:57
I have modified my code as you suggested, I have updated the question above. I am still getting the same error. Do you think it may not be related to my `Promise` anti-pattern? Do you think it my be a `SQLite` issue? — Richard, Oct 17 '16 at 10:16
@Richard can you update the question with the full error, with line numbers? That may help. — Jivings, Oct 17 '16 at 12:48

Javascript Promise Anti-Pattern error

1 Answers1