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

Provide optional Node support for accurate task/microtask behavior #296

Closed
brettz9 opened this issue Apr 2, 2017 · 7 comments
Closed

Provide optional Node support for accurate task/microtask behavior #296

brettz9 opened this issue Apr 2, 2017 · 7 comments
Assignees
Milestone

Comments

@brettz9
Copy link
Collaborator

brettz9 commented Apr 2, 2017

Per a recent README update, there are limitations in the browser, and currently in Node, in regard to task/micro-task timing.

It would seem to me to be compelling to be able to get the fully spec-compliant expected behavior in terms of task/microtask behavior for Node, so that, e.g., transaction expiration would work as expected when timeouts or promises are used. Currently, we prolong transactions in Node which at least allows us to roll back transactions, but it also breaks expected relative timing of transactions given that our asynchronous SQLite calls can take longer than say a user's timeout.

While we ought to be able to fix this in Node* by optionally using a synchronous SQLite implementation such as https://github.com/grumdrig/node-sqlite (probably most cleanly and easily via updating/adapting node-websql to also support WebSQL's synchronous API), such blocking should mostly only make sense in Node by using threads as performance would otherwise be significantly degraded. I don't know if the likes of https://github.com/audreyt/node-webworker-threads or https://nodejs.org/api/cluster.html could help us.

I'm not really sure whether there would need to be any API additions to support this. I'd really be grateful for any feedback on all of this (if you could bear in mind that I don't have a good idea on using threads in Node or their requirements (I'm guessing not all Node hosts support forking child processes) nor have I taken a good look at the spec or the various relevant W3C tests to get a firm grasp on the exact task/microtask expectations of IndexedDB).

* It would also apply to browsers (and help them with rollbacks), but apparently Chrome and Safari never exposed the synchronous WebSQL API, despite it having been spec'd so I believe we're stuck with limitations in both rollbacks and transaction timing on the browser.

@lightsofapollo
Copy link
Contributor

@brettz9 My 2c- I think it's very confusing to people new to idb that your transaction will close if you do not continue to run operations (reads/writes/etc..). I worked on a few large projects that had seriously bad race conditions because of this... For my usecase I am writing tests (/w jest and using this lib to mock out idb).

Might be worth putting up a link to https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB#Adding_data_to_the_database and mentioning the differences more clearly?

@brettz9
Copy link
Collaborator Author

brettz9 commented May 3, 2017

Sure... This is also limiting with chaining promises as they may expire before an operations has a chance to work.

But our code already does close without operations, so it is just a matter of timing it to close with the same timing as it will close in a compliant browser.

While I agree it is perhaps unexpected, I think we should leave it to other sites like MDN to document the ins and outs. Or are you just referring to adding a link in the README portion which mentions this as a known issue in order to clarify what the issue is?

@lightsofapollo
Copy link
Contributor

Or are you just referring to adding a link in the README portion which mentions this as a known issue in order to clarify what the issue is?

^ yeah this- I was not 100% sure what differences there are between what MDN (or the spec) says vs how this lib works.

@brettz9
Copy link
Collaborator Author

brettz9 commented May 3, 2017

Ok, sure. I can look into adding. In the meantime, this test and this one give a good demonstration of the expectations.

We only manage to "pass" these tests in Node by overwriting setTimeout to add another 500ms (fudging which is helpful to make sure there aren't any other issues we're encountering). We can solve this in theory on the Node side by implementing and utilizing the sync WebSQL methods (which were never apparently added to browsers).

In Chrome, we are failing these tests even after overwriting setTimeout, I believe because we can't arbitrarily extend the transaction in browser WebSQL as we can in our custom version of node-websql.

@brettz9
Copy link
Collaborator Author

brettz9 commented May 3, 2017

I've added a commit to clarify the issue further on the README. Please let me know if that helps.

@brettz9
Copy link
Collaborator Author

brettz9 commented Dec 5, 2018

Per nolanlawson/node-websql#28 mentions https://www.npmjs.com/package/better-sqlite3 which claims a very fast and fast synchronous API which may be a good basis for this issue...

@brettz9
Copy link
Collaborator Author

brettz9 commented Mar 24, 2020

Tracking this issue on new package at #8

@brettz9 brettz9 closed this as completed Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants