Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

database.executeSql calls its success callback twice #111

Closed
Typhlosaurus opened this issue Aug 8, 2014 · 8 comments
Closed

database.executeSql calls its success callback twice #111

Typhlosaurus opened this issue Aug 8, 2014 · 8 comments

Comments

@Typhlosaurus
Copy link

The database.executeSql method is implemented in SQLitePlugin.js by passing the provided success and error callbacks both to the sql statement execution and to the transaction. This means that on successful execution the success callback is called first as

callback(transaction, results);

then as:

callback();

On a failure, depending on the nature of the error, either the success then error callbacks will be called or the error callback will be called twice.

These oddities can be avoided by using an explicit transaction and transaction.executeSql.

@nolanlawson
Copy link
Contributor

This sounds like a pretty bad bug. Can you write a unit test to reproduce? Our tests are here and there are instructions in the README for executing them.

@Typhlosaurus
Copy link
Author

OK.

@brodycj
Copy link
Contributor

brodycj commented Aug 8, 2014

This sounds like a pretty bad bug. Can you write a unit test to reproduce?
Our tests are here
https://github.com/brodysoft/Cordova-SQLitePlugin/blob/master/test-www/www/index.html
and there are instructions in the README for executing them.

My idea is to add a unit test that checks that the [first] received
callback does not include the transaction argument. I am happy to take
this one.

Chris

@brodycj
Copy link
Contributor

brodycj commented Aug 11, 2014

Guys, I would like to apply the changes that I have proposed in #117 pretty soon, before submitting to the PhoneGap build. It is basically a one-liner CoffeeScript/JavaScript change. I have only tested it on iOS so far but am absolutely confident it will work for all platforms. Any objections?

@nolanlawson
Copy link
Contributor

I can test it on android 2.3 and 4.4 tonight.

brodycj pushed a commit that referenced this issue Aug 12, 2014
brodycj pushed a commit that referenced this issue Aug 12, 2014
@brodycj
Copy link
Contributor

brodycj commented Aug 12, 2014

I would like to thank you guys both @Typhlosaurus and @nolanlawson for your help on this. @Typhlosaurus we are both very thankful to you that you have identified this bug for us.

I have decided to include a much simpler solution in the interest of timing.

I will use your work to reproduce the other issues in the next few weeks. Thanks again!

@brodycj brodycj closed this as completed Aug 12, 2014
brodycj pushed a commit that referenced this issue Aug 20, 2014
…ng openDatabase()/db.close()/deleteDatabase() callbacks; double db.executeSql() callbacks; deleteDatabase() not reliable); small changes to deleteAndConfirmDeleted() helper function for existing test of sqlitePlugin.deleteDatabase()
@MuhammadImran-04
Copy link

I'm still facing this issue on android. The Plugin version I'm using is: "0.7.15-dev", and cordova version is 5.3.3. Can anyone help?

@brodycj
Copy link
Contributor

brodycj commented Feb 20, 2016

I'm still facing this issue on android. The Plugin version I'm using is: "0.7.15-dev", and cordova version is 5.3.3. Can anyone help?

Unfortunately that is not enough information. Can you demonstrate this with a test program?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants