Skip to content

Mongo driver 4 rc fix duplicate error #95

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Mongo driver 4 rc fix duplicate error #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9ce754d
Fix duplicate error check.
aljones15 Oct 28, 2022
7eec30a
Add more Mongo Driver Errors to exceptions.
aljones15 Oct 28, 2022
91aa399
Add remaining Mongo Error Types.
aljones15 Oct 28, 2022
ea8333d
Remove abstract base classes from exceptions.
aljones15 Nov 1, 2022
3ba0b56
Use assertMongoError.
aljones15 Nov 1, 2022
4ea8316
Update assertMongoError to handle Write & WriteConcern Error.
aljones15 Nov 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Changed
- **BREAKING**: Upgrade to `mongodb: ^4.9.0`
- **BREAKING**: MDBE_ERRORS is now a set.

## 10.1.1 -

Expand Down
19 changes: 13 additions & 6 deletions lib/exceptions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
/*!
* Copyright (c) 2012-2022 Digital Bazaar, Inc. All rights reserved.
*/
export const MDBE_ERROR = 'MongoError';

import {MongoError} from 'mongodb';

// Mongo DB Errors
// thrown inside of a MongoServerError
export const WRITE_ERROR = 'WriteError';
export const BULK_WRITE_ERROR = 'BulkWriteError';
// throw inside of a MongoServerError
export const WRITE_CONCERN_ERROR = 'WriteConcernError';
export const MDBE_ERRORS = [
MDBE_ERROR,

export const MDBE_ERRORS = new Set([
WRITE_ERROR,
BULK_WRITE_ERROR,
WRITE_CONCERN_ERROR
];
]);
Copy link
Member

@dlongley dlongley Oct 31, 2022

Choose a reason for hiding this comment

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

-1 to adding all these, we should only add what we need (and we haven't needed any of these) -- otherwise it's just more to maintain and test.

Copy link
Contributor Author

@aljones15 aljones15 Oct 31, 2022

Choose a reason for hiding this comment

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

That's a good point. There might be an automated way of adding these though. I'll look into it and if not cut them down to something more manageable. I also think not all of these errors are root errors, many of them are the cause of a MongoError I believe.

Copy link
Contributor Author

@aljones15 aljones15 Nov 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Ok so I looked into this today and basically here is the problem: MongoError seems to be nothing more than an abstract class, then MongoError in turn has about 5-6 errors that derive from it, then those 5-6 errors in turn throw the errors listed above and are base classes for a panoply of different errors. So we can pretty safely remove MongoError, but the issue is the driver throws most of the other errors so we probably do need to ensure we know their names.

Copy link
Contributor Author

@aljones15 aljones15 Nov 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

ok so I trimmed this down to just the two errors that are kind of sort of thrown by the driver that are not MongoErrors. Then I added a new function assertMongoError here: 43ab9b0
and here:
94bf529

This method should work as the node mongo driver is now written using typescript so there are a lot of Abstract base classes and inheritance based type checking structures that ultimately result in 3 cases: either you get a MongoError, a WriteError or WriteConcernError. For the most part you only get MongoError, but documents can contain WriteError and MongoError can contain both WriteError or WriteConcernError so there might be edge cases were we're asserting on the cause of a MongoError or on the writeErrors property of MongoError or document.


export const assertMongoError = error =>
(error instanceof MongoError) || MDBE_ERRORS.has(error.name);

export const MDBE_AUTHN_FAILED = 18;
export const MDBE_AUTHZ_FAILED = 13;
export const MDBE_DUPLICATE = 11000;
Expand Down
4 changes: 2 additions & 2 deletions lib/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import * as bedrock from '@bedrock/core';
import crypto from 'node:crypto';
import {
assertMongoError,
MDBE_AUTHN_FAILED,
MDBE_ERRORS,
MDBE_DUPLICATE,
MDBE_DUPLICATE_ON_UPDATE
} from './exceptions.js';
Expand Down Expand Up @@ -227,7 +227,7 @@ export function isDuplicateError(err) {
* not.
*/
export function isDatabaseError(err) {
return (err && MDBE_ERRORS.includes(err.name));
return (err && assertMongoError(err));
}

/**
Expand Down
29 changes: 29 additions & 0 deletions test/mocha/10-api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,35 @@ describe('api', function() {
}
should.not.exist(error);
});
it('should throw DuplicateError on duplicate index', async function() {
let error = null;
try {
await database.openCollections(['test']);
await database.createIndexes([{
collection: 'test',
fields: {id: 1},
options: {unique: true, background: false}
}]);
const record = {
id: database.hash('insert-duplicate')
};
await database.collections.test.insertOne(record);
await database.collections.test.insertOne(record);
} catch(e) {
error = e;
}
should.exist(error);
const assertDbError = database.isDatabaseError(error);
assertDbError.should.equal(
true,
'Expected DuplicateError to be a Databse Error'
);
const assertDuplicateError = database.isDuplicateError(error);
assertDuplicateError.should.equal(
true,
'Expected isDuplicateError to be true'
);
});
});
describe('createGridFSBucket', function() {
it('should create a streaming GridFS bucket instance', async function() {
Expand Down