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

Support React Native 0.76 #9009

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

craftzdog
Copy link

This PR makes createError avoid an error in Hermes.
You can't read error.stack from the manually copied Error object.
See this thread for more details: facebook/hermes#1496

Instead, you have to create a new PouchError with the given error properties.

// so as to allow proper JSON parsing.
var err = new PouchError(error.status, error.name, error.message);
if (reason !== undefined) {
err.reason = reason;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change risk losing non-standard props of the reanimated error message when createError() is called on something other than one of the constants defined in this file? E.g.:

packages/node_modules/pouchdb-find/src/adapters/http/index.js:16:    const pouchError = createError(json);
packages/node_modules/pouchdb-replication/src/replicate.js:299:      fatalError = createError(fatalError);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, I thought it was ok to push as all tests passed.
Yeah, non-standard props will be lost by this change.
So, we should copy props except for stack.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like there's much testing around PouchError or createError(), so maybe it's not surprising that tests were passing. There's also some arcane stuff happening in both of those locations, so it would be good to get more tests in place before making changes. Tests for the property copying is one example.

That said, this PR looks like an improvement and probably it's ok to break behaviour which is weird, undocumented, and probably the result of legacy javsascript distorted by years of refactoring...

Some other examples of behaviour that might be worth testing:

  • it looks like CustomPouchError.prototype = PouchError.prototype propagates PouchError.toString to the result of createError, but this behaviour isn't tested
  • it looks like an instance of PouchError will have property .error set to true, but a CustomPouchError will not, unless it was created from a PouchError. Maybe createError is only meant to be called on instances of PouchError? Either way, this PR currently changes that behaviour.

Maybe CustomPouchError should be a publicly-accessible class? Ref #1167 (comment)

etc.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be better to export PouchError instead of CustomPouchError.

Yeah, it looks like createError is basically meant to be called on instances of PouchError as far as I can see:

 rg createError
pouchdb-adapter-utils/src/updateDoc.js
1:import { createError, REV_CONFLICT } from 'pouchdb-errors';
41:    var err = createError(REV_CONFLICT);

pouchdb-adapter-utils/src/preprocessAttachments.js
8:import { createError, BAD_ARG } from 'pouchdb-errors';
15:    var err = createError(BAD_ARG,

pouchdb-adapter-utils/src/parseDoc.js
6:  createError
52:    return createError(INVALID_REV);
148:        var error = createError(DOC_VALIDATION, key);

pouchdb-changes-filter/src/index.js
4:  createError,
54:      var err = createError(BAD_REQUEST,
72:        return callback(createError(MISSING_DOC,
98:        return callback(createError(MISSING_DOC,

pouchdb-adapter-utils/src/processDocs.js
1:import { createError, MISSING_DOC, REV_CONFLICT } from 'pouchdb-errors';
25:      results[resultsIdx] = createError(MISSING_DOC, 'deleted');
33:      var err = createError(REV_CONFLICT);

pouchdb-replication/src/replicate.js
6:import { createError } from 'pouchdb-errors';
299:      fatalError = createError(fatalError);

pouchdb-utils/src/invalidIdError.js
5:  createError
16:    err = createError(MISSING_ID);
18:    err = createError(INVALID_ID);
20:    err = createError(RESERVED_ID);

pouchdb-replication/src/replicateWrapper.js
4:import { createError, BAD_REQUEST } from 'pouchdb-errors';
26:    throw createError(BAD_REQUEST,

pouchdb-adapter-leveldb-core/src/index.js
54:  createError
374:        return callback(createError(MISSING_DOC, 'missing'));
382:          return callback(createError(MISSING_DOC, "deleted"));
392:          return callback(createError(MISSING_DOC));
476:          var err = createError(MISSING_STUB,
656:            callback(createError(BAD_ARG,
1167:      return callback(createError(NOT_OPEN));
1195:        callback(createError(MISSING_DOC));
1363:        callback(createError(MISSING_DOC));
1396:        return callback(createError(REV_CONFLICT));
1399:        return callback(createError(REV_CONFLICT));
1454:          return callback(createError(MISSING_DOC));
1458:        return callback(createError(REV_CONFLICT));

pouchdb-core/src/adapter.js
36:  createError
210:        return callback(createError(NOT_AN_OBJECT));
221:        return cb(createError(NOT_AN_OBJECT));
225:        return cb(createError(INVALID_REV));
301:          throw createError(REV_CONFLICT);
324:          callback(createError(REV_CONFLICT));
505:        return cb(createError(INVALID_ID));
571:                return cb(createError(INVALID_REV));
576:            return cb(createError(UNKNOWN_ERROR, 'function_clause'));
708:          return callback(createError(MISSING_DOC));
734:          callback(createError(QUERY_PARSE_ERROR,
789:        return callback(createError(MISSING_BULK_DOCS));
795:          return callback(createError(NOT_AN_OBJECT));
798:          return callback(createError(INVALID_REV));
815:        return callback(createError(BAD_REQUEST, attachmentError));
1008:    return callback(createError(UNKNOWN_ERROR, 'Purge is not implemented in the ' + this.adapter + ' adapter.'));
1017:      return callback(createError(MISSING_DOC));

pouchdb-find/src/adapters/http/index.js
1:import { createError, generateErrorFromResponse } from 'pouchdb-errors';
16:    const pouchError = createError(json);

pouchdb-adapter-idb/src/allDocs.js
1:import { createError, IDB_ERROR } from 'pouchdb-errors';
86:      return callback(createError(IDB_ERROR,

pouchdb-errors/src/index.js
46:function createError(error, reason) {
120:  createError,

pouchdb-adapter-idb/src/utils.js
2:import { createError, IDB_ERROR } from 'pouchdb-errors';
24:    callback(createError(IDB_ERROR, message, evt.type));

pouchdb-adapter-http/src/index.js
8:  createError,
696:        return callback(createError(BAD_ARG,

pouchdb-adapter-indexeddb/src/getRevisionTree.js
3:import { createError, MISSING_DOC } from 'pouchdb-errors';
15:      callback(createError(MISSING_DOC));

pouchdb-for-coverage/src/utils.js
43:  createError,
59:  createError,

pouchdb-adapter-indexeddb/src/allDocs.js
3:import { createError, IDB_ERROR } from 'pouchdb-errors';
57:  callback(createError(IDB_ERROR, err.name, err.message));

pouchdb-adapter-idb/src/index.js
24:  createError
346:        err = createError(MISSING_DOC, 'missing');
355:          err = createError(MISSING_DOC, "deleted");
371:          err = createError(MISSING_DOC, 'missing');
455:        callback(createError(MISSING_DOC));
514:        callback(createError(MISSING_DOC));
559:          callback(createError(REV_CONFLICT));
574:        callback(createError(REV_CONFLICT));
614:        callback(createError(MISSING_DOC));
813:    callback(createError(IDB_ERROR, msg));

pouchdb-adapter-idb/src/bulkDocs.js
1:import { createError, MISSING_STUB } from 'pouchdb-errors';
176:        var err = createError(MISSING_STUB,

pouchdb-adapter-indexeddb/src/util.js
3:import { createError, IDB_ERROR } from 'pouchdb-errors';
20:    callback(createError(IDB_ERROR, message, evt.type));

pouchdb-adapter-indexeddb/src/get.js
3:import { createError, MISSING_DOC } from 'pouchdb-errors';
24:      callback(createError(MISSING_DOC, 'missing'));

pouchdb-adapter-indexeddb/src/bulkDocs.js
4:  createError,
85:        newDoc = createError(MISSING_DOC, 'deleted');
91:        newDoc = createError(REV_CONFLICT);
176:      return createError(REV_CONFLICT);
257:            error = createError(MISSING_STUB);
325:        return Promise.reject(createError(BAD_ARG, 'Attachment is not a valid base64 string'));
410:        callback(error || createError(UNKNOWN_ERROR, 'transaction was aborted'));

pouchdb-utils/src/filterChange.js
1:import { createError, BAD_REQUEST } from 'pouchdb-errors';
8:    return createError(BAD_REQUEST, msg);

pouchdb-adapter-indexeddb/src/getLocal.js
1:import { createError, MISSING_DOC } from 'pouchdb-errors';
17:      callback(createError(MISSING_DOC, 'missing'));

pouchdb-adapter-indexeddb/src/getAttachment.js
4:import { createError, MISSING_DOC } from 'pouchdb-errors';
13:    cb(createError(MISSING_DOC, 'missing'));

except for:

pouchdb-replication/src/replicate.js
6:import { createError } from 'pouchdb-errors';
299:      fatalError = createError(fatalError);

pouchdb-find/src/adapters/http/index.js
1:import { createError, generateErrorFromResponse } from 'pouchdb-errors';
16:    const pouchError = createError(json);

So, maybe copying non-standard props is only for these two use cases? 🤔 I'll look into how PouchDB handles these non-standard error props.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another question is: are the const error instances of PouchError ever used without passing them to createError() first? If not, we can probably simplify everything a lot.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess createError is called for getting a correct stack trace thus using the error constants directly is wrong here.
Sounds nice. Can you give me an idea for the simplification?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@craftzdog
Copy link
Author

I think there should now be no breaking changes.

@craftzdog
Copy link
Author

I confirmed that the error constants are not used directly anywhere:

 rg UNAUTHORIZED
pouchdb-errors/src/index.js
21:var UNAUTHORIZED = new PouchError(401, 'unauthorized', "Name or password is incorrect.");
96:  UNAUTHORIZED,

pouchdb-for-coverage/src/errors.js
2:   UNAUTHORIZED,
29:  UNAUTHORIZED,
 rg MISSING_BULK_DOCS
pouchdb-errors/src/index.js
22:var MISSING_BULK_DOCS = new PouchError(400, 'bad_request', "Missing JSON list of 'docs'");
97:  MISSING_BULK_DOCS,

pouchdb-for-coverage/src/errors.js
3:   MISSING_BULK_DOCS,
30:  MISSING_BULK_DOCS,

pouchdb-core/src/adapter.js
27:  MISSING_BULK_DOCS,
789:        return callback(createError(MISSING_BULK_DOCS));
 rg MISSING_DOC
pouchdb-core/src/adapter.js
28:  MISSING_DOC,
308:        if (err.reason === MISSING_DOC.message) {
708:          return callback(createError(MISSING_DOC));
1017:      return callback(createError(MISSING_DOC));

pouchdb-adapter-leveldb-core/src/index.js
49:  MISSING_DOC,
374:        return callback(createError(MISSING_DOC, 'missing'));
382:          return callback(createError(MISSING_DOC, "deleted"));
392:          return callback(createError(MISSING_DOC));
1195:        callback(createError(MISSING_DOC));
1363:        callback(createError(MISSING_DOC));
1454:          return callback(createError(MISSING_DOC));

pouchdb-adapter-idb/src/index.js
21:  MISSING_DOC,
346:        err = createError(MISSING_DOC, 'missing');
355:          err = createError(MISSING_DOC, "deleted");
371:          err = createError(MISSING_DOC, 'missing');
455:        callback(createError(MISSING_DOC));
514:        callback(createError(MISSING_DOC));
614:        callback(createError(MISSING_DOC));

pouchdb-adapter-utils/src/processDocs.js
1:import { createError, MISSING_DOC, REV_CONFLICT } from 'pouchdb-errors';
25:      results[resultsIdx] = createError(MISSING_DOC, 'deleted');

pouchdb-errors/src/index.js
23:var MISSING_DOC = new PouchError(404, 'not_found', 'missing');
98:  MISSING_DOC,

pouchdb-changes-filter/src/index.js
2:  MISSING_DOC,
72:        return callback(createError(MISSING_DOC,
98:        return callback(createError(MISSING_DOC,

pouchdb-adapter-indexeddb/src/getRevisionTree.js
3:import { createError, MISSING_DOC } from 'pouchdb-errors';
15:      callback(createError(MISSING_DOC));

pouchdb-adapter-indexeddb/src/get.js
3:import { createError, MISSING_DOC } from 'pouchdb-errors';
24:      callback(createError(MISSING_DOC, 'missing'));

pouchdb-adapter-indexeddb/src/getAttachment.js
4:import { createError, MISSING_DOC } from 'pouchdb-errors';
13:    cb(createError(MISSING_DOC, 'missing'));

pouchdb-adapter-indexeddb/src/getLocal.js
1:import { createError, MISSING_DOC } from 'pouchdb-errors';
17:      callback(createError(MISSING_DOC, 'missing'));

pouchdb-adapter-indexeddb/src/bulkDocs.js
6:  MISSING_DOC,
85:        newDoc = createError(MISSING_DOC, 'deleted');

pouchdb-for-coverage/src/errors.js
4:   MISSING_DOC,
31:  MISSING_DOC,
 rg REV_CONFLICT
pouchdb-adapter-utils/src/updateDoc.js
1:import { createError, REV_CONFLICT } from 'pouchdb-errors';
41:    var err = createError(REV_CONFLICT);

pouchdb-errors/src/index.js
24:var REV_CONFLICT = new PouchError(409, 'conflict', 'Document update conflict');
99:  REV_CONFLICT,

pouchdb-adapter-utils/src/processDocs.js
1:import { createError, MISSING_DOC, REV_CONFLICT } from 'pouchdb-errors';
33:      var err = createError(REV_CONFLICT);

pouchdb-adapter-idb/src/index.js
22:  REV_CONFLICT,
559:          callback(createError(REV_CONFLICT));
574:        callback(createError(REV_CONFLICT));

pouchdb-adapter-leveldb-core/src/index.js
50:  REV_CONFLICT,
1396:        return callback(createError(REV_CONFLICT));
1399:        return callback(createError(REV_CONFLICT));
1458:        return callback(createError(REV_CONFLICT));

pouchdb-adapter-indexeddb/src/bulkDocs.js
5:  REV_CONFLICT,
91:        newDoc = createError(REV_CONFLICT);
176:      return createError(REV_CONFLICT);

pouchdb-core/src/adapter.js
29:  REV_CONFLICT,
301:          throw createError(REV_CONFLICT);
324:          callback(createError(REV_CONFLICT));

pouchdb-for-coverage/src/errors.js
5:   REV_CONFLICT,
32:  REV_CONFLICT,
 rg INVALID_ID
pouchdb-utils/src/invalidIdError.js
2:  INVALID_ID,
18:    err = createError(INVALID_ID);

pouchdb-for-coverage/src/errors.js
6:   INVALID_ID,
33:  INVALID_ID,

pouchdb-errors/src/index.js
25:var INVALID_ID = new PouchError(400, 'bad_request', '_id field must contain a string');
100:  INVALID_ID,

pouchdb-core/src/adapter.js
30:  INVALID_ID,
505:        return cb(createError(INVALID_ID));
 rg MISSING_ID
pouchdb-errors/src/index.js
26:var MISSING_ID = new PouchError(412, 'missing_id', '_id is required for puts');
101:  MISSING_ID,

pouchdb-utils/src/invalidIdError.js
3:  MISSING_ID,
16:    err = createError(MISSING_ID);

pouchdb-for-coverage/src/errors.js
7:   MISSING_ID,
34:  MISSING_ID,
 rg RESERVED_ID
pouchdb-errors/src/index.js
27:var RESERVED_ID = new PouchError(400, 'bad_request', 'Only reserved document ids may start with underscore.');
102:  RESERVED_ID,

pouchdb-utils/src/invalidIdError.js
4:  RESERVED_ID,
20:    err = createError(RESERVED_ID);

pouchdb-for-coverage/src/errors.js
8:   RESERVED_ID,
35:  RESERVED_ID,
 rg NOT_OPEN
pouchdb-adapter-leveldb-core/src/index.js
51:  NOT_OPEN,
1167:      return callback(createError(NOT_OPEN));

pouchdb-errors/src/index.js
28:var NOT_OPEN = new PouchError(412, 'precondition_failed', 'Database not open');
103:  NOT_OPEN,

pouchdb-for-coverage/src/errors.js
9:   NOT_OPEN,
36:  NOT_OPEN,
 rg UNKNOWN_ERROR
pouchdb-core/src/adapter.js
31:  UNKNOWN_ERROR,
576:            return cb(createError(UNKNOWN_ERROR, 'function_clause'));
1008:    return callback(createError(UNKNOWN_ERROR, 'Purge is not implemented in the ' + this.adapter + ' adapter.'));

pouchdb-for-coverage/src/errors.js
10:   UNKNOWN_ERROR,
37:  UNKNOWN_ERROR,

pouchdb-errors/src/index.js
29:var UNKNOWN_ERROR = new PouchError(500, 'unknown_error', 'Database encountered an unknown error');
67:    err = UNKNOWN_ERROR;
104:  UNKNOWN_ERROR,

pouchdb-adapter-indexeddb/src/bulkDocs.js
9:  UNKNOWN_ERROR
410:        callback(error || createError(UNKNOWN_ERROR, 'transaction was aborted'));
 rg BAD_ARG
pouchdb-adapter-leveldb-core/src/index.js
52:  BAD_ARG,
656:            callback(createError(BAD_ARG,

pouchdb-adapter-utils/src/preprocessAttachments.js
8:import { createError, BAD_ARG } from 'pouchdb-errors';
15:    var err = createError(BAD_ARG,

pouchdb-adapter-http/src/index.js
9:  BAD_ARG,
696:        return callback(createError(BAD_ARG,

pouchdb-adapter-indexeddb/src/bulkDocs.js
8:  BAD_ARG,
325:        return Promise.reject(createError(BAD_ARG, 'Attachment is not a valid base64 string'));

pouchdb-errors/src/index.js
30:var BAD_ARG = new PouchError(500, 'badarg', 'Some query argument is invalid');
105:  BAD_ARG,

pouchdb-for-coverage/src/errors.js
11:   BAD_ARG,
38:  BAD_ARG,
 rg INVALID_REQUEST
pouchdb-for-coverage/src/errors.js
12:   INVALID_REQUEST,
39:  INVALID_REQUEST,

pouchdb-errors/src/index.js
31:var INVALID_REQUEST = new PouchError(400, 'invalid_request', 'Request was invalid');
106:  INVALID_REQUEST,
 rg QUERY_PARSE_ERROR
pouchdb-core/src/adapter.js
32:  QUERY_PARSE_ERROR,
734:          callback(createError(QUERY_PARSE_ERROR,

pouchdb-errors/src/index.js
32:var QUERY_PARSE_ERROR = new PouchError(400, 'query_parse_error', 'Some query parameter is invalid');
107:  QUERY_PARSE_ERROR,

pouchdb-for-coverage/src/errors.js
13:   QUERY_PARSE_ERROR,
40:  QUERY_PARSE_ERROR,
 rg DOC_VALIDATION
pouchdb-errors/src/index.js
33:var DOC_VALIDATION = new PouchError(500, 'doc_validation', 'Bad special document member');
108:  DOC_VALIDATION,

pouchdb-adapter-utils/src/parseDoc.js
4:  DOC_VALIDATION,
148:        var error = createError(DOC_VALIDATION, key);
149:        error.message = DOC_VALIDATION.message + ': ' + key;

pouchdb-for-coverage/src/errors.js
14:   DOC_VALIDATION,
41:  DOC_VALIDATION,
 rg BAD_REQUEST
pouchdb-errors/src/index.js
34:var BAD_REQUEST = new PouchError(400, 'bad_request', 'Something wrong with the request');
109:  BAD_REQUEST,

pouchdb-core/src/adapter.js
33:  BAD_REQUEST,
815:        return callback(createError(BAD_REQUEST, attachmentError));

pouchdb-replication/src/replicateWrapper.js
4:import { createError, BAD_REQUEST } from 'pouchdb-errors';
26:    throw createError(BAD_REQUEST,

pouchdb-changes-filter/src/index.js
3:  BAD_REQUEST,
54:      var err = createError(BAD_REQUEST,

pouchdb-utils/src/filterChange.js
1:import { createError, BAD_REQUEST } from 'pouchdb-errors';
8:    return createError(BAD_REQUEST, msg);

pouchdb-for-coverage/src/errors.js
15:   BAD_REQUEST,
42:  BAD_REQUEST,
 rg NOT_AN_OBJECT
pouchdb-core/src/adapter.js
34:  NOT_AN_OBJECT,
210:        return callback(createError(NOT_AN_OBJECT));
221:        return cb(createError(NOT_AN_OBJECT));
795:          return callback(createError(NOT_AN_OBJECT));

pouchdb-errors/src/index.js
35:var NOT_AN_OBJECT = new PouchError(400, 'bad_request', 'Document must be a JSON object');
110:  NOT_AN_OBJECT,

pouchdb-for-coverage/src/errors.js
16:   NOT_AN_OBJECT,
43:  NOT_AN_OBJECT,
 rg DB_MISSING
pouchdb-errors/src/index.js
36:var DB_MISSING = new PouchError(404, 'not_found', 'Database not found');
111:  DB_MISSING,

pouchdb-for-coverage/src/errors.js
17:   DB_MISSING,
44:  DB_MISSING,
 rg IDB_ERROR
pouchdb-for-coverage/src/errors.js
24:   IDB_ERROR,
51:  IDB_ERROR,

pouchdb-adapter-idb/src/allDocs.js
1:import { createError, IDB_ERROR } from 'pouchdb-errors';
86:      return callback(createError(IDB_ERROR,

pouchdb-adapter-idb/src/index.js
23:  IDB_ERROR,
813:    callback(createError(IDB_ERROR, msg));

pouchdb-adapter-indexeddb/src/allDocs.js
3:import { createError, IDB_ERROR } from 'pouchdb-errors';
57:  callback(createError(IDB_ERROR, err.name, err.message));

pouchdb-errors/src/index.js
37:var IDB_ERROR = new PouchError(500, 'indexed_db_went_bad', 'unknown');
118:  IDB_ERROR,

pouchdb-adapter-idb/src/utils.js
2:import { createError, IDB_ERROR } from 'pouchdb-errors';
24:    callback(createError(IDB_ERROR, message, evt.type));

pouchdb-adapter-indexeddb/src/util.js
3:import { createError, IDB_ERROR } from 'pouchdb-errors';
20:    callback(createError(IDB_ERROR, message, evt.type));
 rg WSQ_ERROR
pouchdb-errors/src/index.js
38:var WSQ_ERROR = new PouchError(500, 'web_sql_went_bad', 'unknown');
112:  WSQ_ERROR,

pouchdb-for-coverage/src/errors.js
18:   WSQ_ERROR,
45:  WSQ_ERROR,
 rg LDB_ERROR
pouchdb-errors/src/index.js
39:var LDB_ERROR = new PouchError(500, 'levelDB_went_went_bad', 'unknown');
113:  LDB_ERROR,

pouchdb-for-coverage/src/errors.js
19:   LDB_ERROR,
46:  LDB_ERROR,
 rg FORBIDDEN
pouchdb-for-coverage/src/errors.js
20:   FORBIDDEN,
47:  FORBIDDEN,

pouchdb-errors/src/index.js
40:var FORBIDDEN = new PouchError(403, 'forbidden', 'Forbidden by design doc validate_doc_update function');
114:  FORBIDDEN,
 rg INVALID_REV
pouchdb-adapter-utils/src/parseDoc.js
5:  INVALID_REV,
52:    return createError(INVALID_REV);

pouchdb-core/src/adapter.js
35:  INVALID_REV,
225:        return cb(createError(INVALID_REV));
571:                return cb(createError(INVALID_REV));
798:          return callback(createError(INVALID_REV));

pouchdb-for-coverage/src/errors.js
21:   INVALID_REV,
48:  INVALID_REV,

pouchdb-errors/src/index.js
41:var INVALID_REV = new PouchError(400, 'bad_request', 'Invalid rev format');
115:  INVALID_REV,
 rg FILE_EXISTS
pouchdb-errors/src/index.js
42:var FILE_EXISTS = new PouchError(412, 'file_exists', 'The database could not be created, the file already exists.');
116:  FILE_EXISTS,

pouchdb-for-coverage/src/errors.js
22:   FILE_EXISTS,
49:  FILE_EXISTS,
 rg MISSING_STUB
pouchdb-adapter-leveldb-core/src/index.js
53:  MISSING_STUB,
476:          var err = createError(MISSING_STUB,

pouchdb-for-coverage/src/errors.js
23:   MISSING_STUB,
50:  MISSING_STUB,

pouchdb-adapter-idb/src/bulkDocs.js
1:import { createError, MISSING_STUB } from 'pouchdb-errors';
176:        var err = createError(MISSING_STUB,

pouchdb-errors/src/index.js
43:var MISSING_STUB = new PouchError(412, 'missing_stub', 'A pre-existing attachment stub wasn\'t found');
117:  MISSING_STUB,

pouchdb-adapter-indexeddb/src/bulkDocs.js
7:  MISSING_STUB,
257:            error = createError(MISSING_STUB);
 rg INVALID_URL
pouchdb-errors/src/index.js
44:var INVALID_URL = new PouchError(413, 'invalid_url', 'Provided URL is invalid');
119:  INVALID_URL,

pouchdb-for-coverage/src/errors.js
25:   INVALID_URL
52:  INVALID_URL

@craftzdog
Copy link
Author

it looks like an instance of PouchError will have property .error set to true, but a CustomPouchError will not, unless it was created from a PouchError.

I couldn't find lines where PouchDB explicitly checks if an error instance has .error === true. So, I don't know what's the purpose of this prop in PouchError at the moment.

@craftzdog
Copy link
Author

Added some unit tests

@alxndrsn
Copy link
Member

alxndrsn commented Dec 6, 2024

I guess createError is called for getting a correct stack trace thus using the error constants directly is wrong here.
Sounds nice. Can you give me an idea for the simplification?

Maybe something like removing createError() and replacing:

class PouchError extends Error {
  constructor(status, type, reason) {
    super(reason);
    this.name = type;
    this.status = status;
  }

  toString() {
    return JSON.stringify({
      status: this.status,
      name: this.name,
      message: this.message,
      reason: this.reason,
    }); 
  }

  static create(status, type, generalReason) {
    return reason => new PouchError(status, type, reason ?? generalReason);
  }
}

const UNAUTHORIZED = PouchError.create(401, 'unauthorized', "Name or password is incorrect.");
const MISSING_BULK_DOCS = PouchError.create(400, 'bad_request', "Missing JSON list of 'docs'");
const MISSING_DOC = PouchError.create(404, 'not_found', 'missing');
const REV_CONFLICT = PouchError.create(409, 'conflict', 'Document update conflict');
const INVALID_ID = PouchError.create(400, 'bad_request', '_id field must contain a string');
const MISSING_ID = PouchError.create(412, 'missing_id', '_id is required for puts');
const RESERVED_ID = PouchError.create(400, 'bad_request', 'Only reserved document ids may start with underscore.');
const NOT_OPEN = PouchError.create(412, 'precondition_failed', 'Database not open');
const UNKNOWN_ERROR = PouchError.create(500, 'unknown_error', 'Database encountered an unknown error');
const BAD_ARG = PouchError.create(500, 'badarg', 'Some query argument is invalid');
const INVALID_REQUEST = PouchError.create(400, 'invalid_request', 'Request was invalid');
const QUERY_PARSE_ERROR = PouchError.create(400, 'query_parse_error', 'Some query parameter is invalid');
const DOC_VALIDATION = PouchError.create(500, 'doc_validation', 'Bad special document member');
const BAD_REQUEST = PouchError.create(400, 'bad_request', 'Something wrong with the request');
const NOT_AN_OBJECT = PouchError.create(400, 'bad_request', 'Document must be a JSON object');
const DB_MISSING = PouchError.create(404, 'not_found', 'Database not found');
const IDB_ERROR = PouchError.create(500, 'indexed_db_went_bad', 'unknown');
const WSQ_ERROR = PouchError.create(500, 'web_sql_went_bad', 'unknown');
const LDB_ERROR = PouchError.create(500, 'levelDB_went_went_bad', 'unknown');
const FORBIDDEN = PouchError.create(403, 'forbidden', 'Forbidden by design doc validate_doc_update function');
const INVALID_REV = PouchError.create(400, 'bad_request', 'Invalid rev format');
const FILE_EXISTS = PouchError.create(412, 'file_exists', 'The database could not be created, the file already exists.');
const MISSING_STUB = PouchError.create(412, 'missing_stub', 'A pre-existing attachment stub wasn\'t found');
const INVALID_URL = PouchError.create(413, 'invalid_url', 'Provided URL is invalid');

and uses of createError() from

return Promise.reject(createError(BAD_ARG, 'Attachment is not a valid base64 string'));

to

return Promise.reject(BAD_ARG('Attachment is not a valid base64 string'));

It could also be possible to keep the existing function signature for createError() if that is preferred, but similarly removing the extra layer of PouchError > CustomPouchError.

@craftzdog
Copy link
Author

Changing predefined errors into functions affect lines like this one:

 rg MISSING_DOC
pouchdb-core/src/adapter.js
28:  MISSING_DOC,
308:        if (err.reason === MISSING_DOC.message) {

@craftzdog
Copy link
Author

Would it be possible to merge this PR since refactoring the error module would look like another work?

@alxndrsn
Copy link
Member

Would it be possible to merge this PR since refactoring the error module would look like another work?

I've tried refactoring at https://github.com/pouchdb/pouchdb/pull/9028/files; would this fix for React Native?

@alxndrsn
Copy link
Member

I couldn't find lines where PouchDB explicitly checks if an error instance has .error === true. So, I don't know what's the purpose of this prop in PouchError at the moment.

Here's an example of .error being used:

if (err || (results[0] && results[0].error)) {

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

Successfully merging this pull request may close these issues.

2 participants