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

Interface: Error #1

Open
AGBrown opened this issue Apr 28, 2015 · 5 comments
Open

Interface: Error #1

AGBrown opened this issue Apr 28, 2015 · 5 comments

Comments

@AGBrown
Copy link
Owner

AGBrown commented Apr 28, 2015

What is the shape of an error (as passed to a callback/promise)?

  1. Which these properties below are optional/mandatory and what do they mean?
  2. What other properties exist on error?
  3. Is it method-dependent?

Proposed shape:

{
    status: number;
    error: string;
    reason: string;
}
@nolanlawson
Copy link

Most of the errors do look like this. It's not guaranteed, though.

There are certain cases where the errors are meaningful and we expect people to handle them themselves. In those cases you're supposed to look for the status member, e.g. err.status === 404 or err.status === 409.

@AGBrown
Copy link
Owner Author

AGBrown commented Apr 29, 2015

Thanks @nolanlawson . Is it predictable per-method which type of error comes back, or is it more to do with things like dealing with remote/local databases?

@marten-de-vries
Copy link

@AGBrown Some good examples of 'other' errors are in the PouchDB constructor: https://github.com/pouchdb/pouchdb/blob/master/lib/constructor.js#L59 . On top of that, there are native adapter errors. E.g.:

marten@procyon:~/git/pouchdb$ node
> var PouchDB = require('./')
undefined
> var db = new PouchDB('/djfkldsjf/jdfklsdjf/jdkljf')
undefined
> db.allDocs()
[object Object]
> Possibly unhandled OpenError: IO error: /djfkldsjf/jdfklsdjf/jdkljf/LOCK: No such file or directory
    at /home/marten/git/pouchdb/node_modules/levelup/lib/levelup.js:114:34

Oh, and bulkDocs() doesn't immediately fail on errors, but instead returns an array of all results, some of which can be errors in its object. See the website for an example on that: http://pouchdb.com/api.html#batch_create

That's the ones I know but should get you started.

@AGBrown
Copy link
Owner Author

AGBrown commented Apr 29, 2015

@marten-de-vries, thank you I will take a look at those examples.

As background for this discussion: the old pouch.d.ts (for 0.1) defined a shape (an interface) for errors, and then set that as the shape used by many of the callbacks (including allDocs - examples below).

My thinking has now evolved based on all the input so far. I think we are trying to work out if there is a predictable shape for an error, either for:

  1. every method call;
  2. per-method call (i.e. method name);
  3. dependent on input options (implying a possible typescript method overload); or
  4. maybe by defining other "partitions" (e.g. a different interface for remote/local dbs) in the same way I am currently splitting out the promise- or callback-style set of interfaces to keep method overloading simpler.
  5. a combination of 2-4 above is also an option

Thus far, as per your and @nolanlawson 's comments, I can now see that (1) would be misleading and worse than to specify the error type as any. I would however prefer not to use any if it is feasible not to. While this is fine it doesn't really help the developer using the d.ts as it doesn't give them any hints as to what properties the error object may have on it.

This is the example code I mentioned above from the old d.ts. This isn't good for the developer as when running your code the error you got would presumably be passed to the callback. The developer would then expect the status property, for example, which would actually be undefined.

interface PouchError {
    status: number;
    error: string;
    reason: string;
}
interface PouchApi {
    ...
    allDocs(opts: PouchAllDocsOptions, callback: (err: PouchError, res: PouchAllDocsResponse) => void): void;
    allDocs(callback: (err: PouchError, res: PouchAllDocsResponse) => void): void;
}

@AGBrown
Copy link
Owner Author

AGBrown commented May 11, 2015

I have now reviewed pouchdb/lib/deps/errors.js and restructured the error interfaces accordingly in 56a04ad. The basic error interface is now used in the standard callback and promise responses. I have also used PouchError on the response to bulkDocs in 39a13c6. I think this is an improvement but, as always, welcome input.

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

3 participants