2

I'm working in a NodeJS project, this project I decided to change the way I'm doing it because this way wasn't working, let me try to explain it.

I need to insert data into a SQL Server DB, so I did a function insertOffice() this function opens a connection using Tedious, then fetchs data to an url with data from an array data2 to load coords, and then with this coords creates an object, then inserts this object into a DB. When inserting only one part of my data2 array, it works, by only sendind data[0] it adds:

{
  latjson: 1,
  lonjson: 1,
  idoficina: "1",
}

But I want to insert both of the parts of my array, changing data2[0] to data2[index], to be able to insert all my array, so I tried creating another function functionLooper()that loops insertOffice() to insert my data from my array data2. I builded this little code to learn how to loop a function, this prints index that is the value I use for bringing idoficina.

As you can see functionLooper() runs the code twice, so it can read fully data2 array, I have this little code that works with the same logic, I builded my full code using this:

function insertOffice(index) {
    console.log(index);
}

function functionLooper() {
    for (let i = 0; i < 5; i++) {
        let response = insertOffice(i);
    }
}

functionLooper();

This prints:

0
1
2
3
4

So my code it's supposed to send index

I'm expecting my code to loop my insertOffice() and being able to insert my objects, the issue is that this doesn't seems to work as I am getting this error:

C:\...\node_modules\tedious\lib\connection.js:993
      throw new _errors.ConnectionError('`.connect` can not be called on a Connection in `' + this.state.name + '` state.');
            ^

ConnectionError: `.connect` can not be called on a Connection in `Connecting` state.

this is my code:

var config = {
    ....
 };

const data2 = [
    ...
 ];

var connection = new Connection(config);

function insertOffice(index) {
    console.log(index)
    connection.on("connect", function (err) {
        console.log("Successful connection");
    });
    connection.connect();

    const request = new Request(
        "EXEC SPInsert @Data1, ... ",
        function (err) {
            if (err) {
                console.log("Couldn't insert, " + err);
            } else {
                console.log("Inserted")
            }
        }
    );
    console.log(myObject.Id_Oficina)
    request.addParameter("Data1", TYPES.SmallInt, myObject.Id_Oficina);
    request.on("row", function (columns) {
        columns.forEach(function (column) {
            if (column.value === null) {
                console.log("NULL");
            } else {
                console.log("Product id of inserted item is " + column.value);
            }
        });
    });
    request.on("requestCompleted", function () {
        connection.close();
    });
    connection.execSql(request);
}

function functionLooper() {
    for (let i = 0; i < 2; i++) {
        let response = insertOffice(i);
    }
}

functionLooper();

I do not know if this is the right way to do it (looping the inserting function insertOffice()twice), if you know a better way to do it and if you could show me how in an example using a similar code to mine, would really appreciate it.

Martín JF
  • 152
  • 2
  • 14
  • Can you check if connection is already in an open state prior to calling connection.connect(). Perhaps setting a flag in connection.on("connect", function (err) {}. – Ross Bush Dec 15 '21 at 16:33
  • @RossBush How can I check that mate? Because when I run my code without the loop it connects, but in the loop it doesn't – Martín JF Dec 15 '21 at 16:34
  • @RossBush any other way I could fix it? – Martín JF Dec 15 '21 at 19:37
  • Do you ever get the "Successful connection" message? – RBarryYoung Dec 15 '21 at 19:55
  • @RBarryYoung no, only when I run it without the loop – Martín JF Dec 15 '21 at 19:56
  • Does the `console.log(index);` happen for any of the indexes? – RBarryYoung Dec 15 '21 at 19:59
  • @RBarryYoung yes mate, my ```console.log(index);``` prints ```0 1``` – Martín JF Dec 15 '21 at 20:00
  • 1
    Then I think that you need to create a new connection every time you enter your `insertOffice(..)` method. It appears that it is taking a while for the connection to resolve, and you are re-entering and trying to reuse the same connection, before it has finished connecting. So you either need to wait at some point, or else create new Connections every time. – RBarryYoung Dec 15 '21 at 20:10

2 Answers2

2

You're approaching an asynchronous problem as if it's a synchronous one. You're also making your life a bit harder by mixing event based async tasks with promise based ones.

For example, connection.connect() is asynchronous (meaning that it doesn't finish all its work before the next lines of code is executed), it is only done when connection emits the connect event. So the trigger for starting the processing of your data should not be started until this event is fired.

For each of the events in your loop they are not running one at a time but all at the same time because the fetch() is a promise (asynchronous) it doesn't complete before the next iteration of the loop. In some cases it may have even finished before the database connection is ready, meaning the code execution has moved on to DB requests before the connection to the database is established.

To allow your code to be as manageable as possible you should aim to "promisify" the connection / requests so that you can then write an entirely promise based program, rather than mixing promises and events (which will be pretty tricky to manage - but is possible).

For example:

const connection = new Connection(config);
// turn the connection event into a promise
function connect() {
 return new Promise((resolve, reject) => {
  connection.once('connect', (err) => err ? reject(err) : resolve(connection));
  connection.connect()
 });
}

// insert your data once the connection is ready and then close it when all the work is done
function insertOffices() {
  connect().then((conn) => {
    // connection is ready I can do what I want
    // NB: Make sure you return a promise here otherwise the connection.close() call will fire before it's done
  }).then(() => {
    connection.close();
  });
}

The same approach can be taken to "promisify" the inserts.

// turn a DB request into a promise
function request(conn) {
  return new Promise((resolve, reject) => {
    const request = new Request(...);
    request.once('error', reject);
    request.once('requestCompleted', resolve);
    conn.execSql(request);
  });
}

This can then be combined to perform a loop where it's executed one at a time:

function doInserts() {
  return connect().then((conn) => {
    // create a "chain" of promises that execute one after the other
    let inserts = Promise.resolve();
    for (let i = 0; i < limit; i++) {
        inserts = inserts.then(() => request(conn));
    }
    return inserts;
  }).then(() => connection.close())
}

or in parallel:

function doInserts() {
  return connect().then((conn) => {
    // create an array of promises that all execute independently
    // NB - this probably won't work currently because it would need
    // multiple connections to work (rather than one)
    let inserts = [];
    for (let i = 0; i < limit; i++) {
        inserts.push(request(conn));
    }
    return Promise.all(inserts);
  }).then(() => connection.close())
}
Dan Hensby
  • 1,118
  • 6
  • 11
  • This is what I suspected was going on (as mentioned in my comment above), unfortunately I am not familiar enough with the technologies (both Tedious and JS events & promises) to provide this excellent answer. – RBarryYoung Dec 15 '21 at 21:04
  • @DanHensby mate, thanks for your amazing answer, but now when I try to insert only one data (before adding the loop functionallity) it gives me this error: Error: Requests can only be made in the LoggedIn state, not the Final state, what can it be? I did what you said until the loop – Martín JF Dec 15 '21 at 23:18
  • You're getting that error when you're only trying to make one request to the DB? That error is usually when you're trying to make a request before the last one is finished. I will point out that none of my above code is tested, so there may be problems with it, but it's the general approach that is required to solve your problem. – Dan Hensby Dec 16 '21 at 09:27
  • @DanHensby yep, the error is when I'm only trying to make one request :c – Martín JF Dec 16 '21 at 15:02
1

Finally I could fix it, I'm sharing my code for everyone to could use it and do multiple inserts, thanks to Dan Hensby, I didn't do it his way but used part of what he said, thanks to RbarryYoung and MichaelSun90 who told me how, just what I did was changing my

var connection = new Connection(config);

to run inside my

function insertOffice(index) { ... }

Looking like this:

function insertOffice(index) {
    var connection = new Connection(config);
    ....
}
Martín JF
  • 152
  • 2
  • 14
  • 1
    Heh. You know, this is exactly what I said in the comments above "*create a new connection every time you enter your `insertOffice(..)` method.* – RBarryYoung Dec 20 '21 at 17:39