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

Infinite loop on driver open fail #511

Closed
marcucio opened this issue Jan 26, 2016 · 8 comments
Closed

Infinite loop on driver open fail #511

marcucio opened this issue Jan 26, 2016 · 8 comments

Comments

@marcucio
Copy link
Contributor

fix for bug where we were going into an infinite loop if the open database failed (saw this on BB10 cordova app)

I don't think we should call _initStorage again since it just failed

fixed in pull request: #510

@thgreasi
Copy link
Member

Perhaps we could remove this fallback and just reject the promise. The mechanism in the main localforage.js module can handle the fallback to the next preferred driver. I had seen that earlier, before the es6 refactor and started a discussion about it in an other issue, but can't find it right now. This was probably used before the fallback-on-fail mechanism was introduced, to prevent some missconfigurations.

@marcucio
Copy link
Contributor Author

@thgreasi so your saying we might not have to do self.setDriver(self.LOCALSTORAGE), it looks like indexdb does it this way (just resolves it):

        function ignoreErrors() {
            // Don't handle errors here,
            // just makes sure related localForages aren't pending.
            return Promise.resolve();
        }

        for (var j = 0; j < dbContext.forages.length; j++) {
            var forage = dbContext.forages[j];
            if (forage !== self) { // Don't wait for itself...
                initPromises.push(forage._initReady().catch(ignoreErrors));
            }
        }

@thgreasi
Copy link
Member

Exactly. I think that catch should just do a reject().
Any chance you can provide a test case or a code example with the infinite
loop?

Thodoris Greasidis
Computer, Networking &
Communications Engineer

@marcucio
Copy link
Contributor Author

yes, it is easy to repro:
in websql.js replace:

dbInfo.db = openDatabase(dbInfo.name, String(dbInfo.version), dbInfo.description, dbInfo.size);

with:

throw new Error('quota exceded');

results in this:

Running "mocha:unit" (mocha) task
Testing: http://localhost:9999/test/test.component.html


  ․
Running PhantomJS...ERROR
>> PhantomJS has crashed. Please read the crash reporting guide at https://github.com/ariya/phantomjs/wiki/Crash-Reporting and file a bug report at https://github.com/ariya/phantomjs/issues/new with the crash dump file attached: /tmp/4966558E-41D1-407A-8444-C4363882E5FC.dmp 0 [ 'PhantomJS has crashed. Please read the crash reporting guide at https://github.com/ariya/phantomjs/wiki/Crash-Reporting and file a bug report at https://github.com/ariya/phantomjs/issues/new with the crash dump file attached: /tmp/4966558E-41D1-407A-8444-C4363882E5FC.dmp' ]
Warning: PhantomJS exited unexpectedly with exit code 2. Use --force to continue.

Aborted due to warnings.
localForage mike$ 

@thgreasi
Copy link
Member

I guess we will have to add a test with since monkey patching...

@marcucio
Copy link
Contributor Author

We might be able to redefine window.openDatabase to ‘undefined’ so that it throws an exception during a test, not sure I will have to play around with it. I’ll be pretty busy today, maybe tomorrow I can look into it

Mike Marcucio
marcucio.com http://marcucio.com/
mike@marcucio.com mailto:mike@marcucio.com

blog http://blog.getitdoneapp.com/ | twitter https://twitter.com/GetItDoneBlog | facebook https://www.facebook.com/GetItDoneTasks | google+ https://plus.google.com/u/0/b/116698933997097774721/+Marcucio/posts

On Jan 27, 2016, at 1:28 AM, Thodoris Greasidis notifications@github.com wrote:

I guess we will have to add a test with since monkey patching...

On Tue, Jan 26, 2016, 21:13 Mike notifications@github.com wrote:

yes, it is easy to repro:
in websql.js replace:

dbInfo.db = openDatabase(dbInfo.name, String(dbInfo.version), dbInfo.description, dbInfo.size);
``
with:

throw new Error('quota exceded');

results in this:

Running "mocha:unit" (mocha) task
Testing: http://localhost:9999/test/test.component.html


Running PhantomJS...ERROR

PhantomJS has crashed. Please read the crash reporting guide at
https://github.com/ariya/phantomjs/wiki/Crash-Reporting and file a bug
report at https://github.com/ariya/phantomjs/issues/new with the crash
dump file attached: /tmp/4966558E-41D1-407A-8444-C4363882E5FC.dmp 0 [
'PhantomJS has crashed. Please read the crash reporting guide at
https://github.com/ariya/phantomjs/wiki/Crash-Reporting and file a bug
report at https://github.com/ariya/phantomjs/issues/new with the crash
dump file attached: /tmp/4966558E-41D1-407A-8444-C4363882E5FC.dmp' ]
Warning: PhantomJS exited unexpectedly with exit code 2. Use --force to
continue.

Aborted due to warnings.
localForage mike$


Reply to this email directly or view it on GitHub
#511 (comment)
.

Thodoris Greasidis
Computer, Networking &
Communications Engineer

Reply to this email directly or view it on GitHub #511 (comment).

@thgreasi
Copy link
Member

Or even better to a function that throws an error.
Something similar is done in test.nodriver.html.
How about placing the new tests in a new file named test.faultydrivers.html or test.drivererrors.html?

marcucio added a commit to marcucio/localForage that referenced this issue Jan 28, 2016
tofumatt pushed a commit that referenced this issue Feb 3, 2016
…abase failed (saw this on BB10 cordova app)

added tests for #511
tofumatt pushed a commit that referenced this issue Feb 3, 2016
@tofumatt
Copy link
Member

tofumatt commented Feb 3, 2016

Merged in #517 with some tidying.

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

3 participants