jeremydaly / serverless-mysql

A module for managing MySQL connections at SERVERLESS scale
MIT License
1.2k stars 83 forks source link

Add: transaction's end function #103

Closed code-xhyun closed 1 year ago

code-xhyun commented 3 years ago
const transaction = () => {

    let queries = [] // keep track of queries
    let rollback = () => {} // default rollback event

    return {
      query: function(...args) {
        if (typeof args[0] === 'function') {
          queries.push(args[0])
        } else {
          queries.push(() => [...args])
        }
        return this
      },
      rollback: function(fn) {
        if (typeof fn === 'function') { rollback = fn }
        return this
      },
      commit: async function() { return await commit(queries,rollback) },
      end: function(){ return end() } //  <-- i want add this!!!!!!! 
    }
  }

i want add this function like use this

let results = await mysql.transaction()
  .query('INSERT INTO table (x) VALUES(?)', [1])
  .query('UPDATE table SET x = 1')
  .rollback(e => { /* do something with the error */ }) 
  .commit() 
  .end() // <-- THIS
trasherdk commented 3 years ago

Shouldn't there be a catch in there somewhere?

What is wrong with this way? ( snippet from #85 )

const transaction = await db.transaction()

try {
  transaction.query(() => ["INSERT INTO items (created_at) VALUES(?)", [new Date()]])
  transaction.commit()
catch (e) {
  transaction.rollback()
}

transaction.end()
code-xhyun commented 3 years ago

@trasherdk

If you want to be like you, We have to write it like this

const transaction = await db.transaction()

try {
  transaction.query(() => ["INSERT INTO items (created_at) VALUES(?)", [new Date()]])
  transaction.commit()
catch (e) {
  transaction.rollback()
}

/*transaction.end()*/ //<-- we can't wirte this  transaction.end is not a function
db.end() //<-- We have to write it like this

so we need to add end function in transaction function

trasherdk commented 3 years ago

Okay, still... Should your rollback() not be wrapped in a catch() ?

code-xhyun commented 3 years ago

i think you don't need rollback wrapped in a catch() This will receive the error object, allowing you to add additional logging or perform some other action

trasherdk commented 3 years ago

Wut? I thought rollback() actually did a rollback. Isn't that the hole purpose of that command?

code-xhyun commented 3 years ago

I don't understand what you're trying to say.

The reason I open this issue is because "DBpool"disconnect smoothly with end()

trasherdk commented 3 years ago

In your example snippet above

let results = await mysql.transaction()
  .query('INSERT INTO table (x) VALUES(?)', [1])
  .query('UPDATE table SET x = 1')
  .rollback(e => { /* do something with the error */ }) 
  .commit() 
  .end()

you have a rollback() before a commit(). So, if rollback() is actually rolling back data, what's left for commit() to do? That was what I was wondering about.

code-xhyun commented 3 years ago

It is just an example In the example, the focus is on the end function.

also Don't you need a commit() to run query() and rollback() on transaction?

dan-kwiat commented 2 years ago

Wut? I thought rollback() actually did a rollback. Isn't that the hole purpose of that command?

@trasherdk I was confused too - turns out the rollback method doesn't trigger a rollback, it's just a callback helper which gets called if your transaction rolls back.

naorpeled commented 1 year ago

Hey @trasherdk, thanks for opening this issue!

I personally don't expect a transaction to have an end command, it's a part of the DB connection manager.

db.end() should do the trick like you mentioned here.

I'll be closing this issue because I don't see a reason for this to be implemented. Feel free to ping me if you need any further help.