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

Are errors handled correctly? #153

Open
webzwo0i opened this issue Dec 12, 2020 · 0 comments
Open

Are errors handled correctly? #153

webzwo0i opened this issue Dec 12, 2020 · 0 comments

Comments

@webzwo0i
Copy link
Member

That could be a long road, but I'm not sure what will happen exactly when the database returns an error. Normally we expect all queries to return in time without errors and I think the case of the whole database is failing (e.g. shutdown/no network) is covered. But in theory, especially with implemented timeout logic, all cases in etherpad, where we access the database (no matter if it's a get or set) should have test coverage for their error callbacks.

We could have redo logic in some cases or disconnect the clients in other cases and for failing set operations we must ensure that the pad stays intact. The latter can be hard, as clients get ACCEPT_COMMIT from the read cache, while the value is not in the db yet. When a instance is shutdown (does it flush all values before exiting?) or restarted (clients are at revision X, reconnect logic kicks in and the server is at revision X minus some revisions - it's okay if the some of the last revisions are lost, but the clients should be able to reconnect gracefully).

I'm especially concerned of cases, where revisions are stored, but the atext is not yet stored or vice versa. (probably both set operations are included in the same bulk operation - but when we disable writeInterval and write values immediately to db, as soon as pooling is implemented, it could mean that one value is written, while the other is not.)

While I don't think it is very critical, it affects stability and I suspect most of the error handling is not covered during our tests.

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

1 participant