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

Validation errors are not consistent for local and remote databases. #4

Open
tohagan opened this issue Dec 7, 2015 · 9 comments
Open

Comments

@tohagan
Copy link

tohagan commented Dec 7, 2015

From my project unit tests I've identified that in order to get identical errors returned from a local or remote database you need the following code change

if (typeof e.unauthorized !== "undefined") {
  throw {
    name: "unauthorized",
    message: 'Name or password is incorrect.',
    reason: e.unauthorized,
    status: 401
  };
} else if (typeof e.forbidden !== "undefined") {
  throw {
    name: "forbidden",
    message: 'Forbidden by design doc validate_doc_update function',
    reason: e.forbidden,
    status: 403
  };
  • renamed message field to reason
  • added new message field text
@marten-de-vries
Copy link
Member

Nice find. Hmm, is reason new in PouchDB? I thought it converted reason -> name and error -> message always. Unless that changed this might actually be a PouchDB bug?

@nolanlawson
Copy link
Member

@marten-de-vries that is a bug. reason should not be in there.

@tohagan
Copy link
Author

tohagan commented Dec 13, 2015

Might be a rather nasty breaking change to PouchDB to alter the error fields as there is sure to be tons of code that depends on it ... what do you think? I needed consistency so that I can write an app that switches data source to be: remote, local (live sync on/off) and db API is consistent. I'm also fetching ddocs from server.

@marten-de-vries
Copy link
Member

@tohagan Any reason why you can't just use err.name? The value should be the same as err.reason, and it's what PouchDB recommends as well.

@marten-de-vries
Copy link
Member

I just ported over the old pouchdb-validation test suite, seems like this is indeed a PouchDB bug. And that it's not as easy to circumvent as I assumed in my last post. Going to open a PouchDB bug.

@marten-de-vries
Copy link
Member

As soon as pouchdb/pouchdb#4670 is fixed, the test I mentioned in that bug can be re-enabled and then we can close this.

@tohagan
Copy link
Author

tohagan commented Dec 25, 2015

Thanks for all your test suite porting and chasing this issue Marten. Really appreciated.

@SCdF
Copy link

SCdF commented Mar 15, 2017

@marten-de-vries looks like the linked ticket (which was a dupe of pouchdb/pouchdb#5214) has now been fixed.

@marten-de-vries
Copy link
Member

marten-de-vries commented Mar 15, 2017

That should solve this issue without further work being required. Before closing though, I think this test (well, the equivalent in the latest master branch) should be re-enabled, so we can be sure:

https://github.com/pouchdb/pouchdb-validation/pull/5/files#diff-fb0d18ca230e188e02029b32dbb851d8R28

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