Skip to content

fix: server crashes on MongoDB GeoJSON error instead of throwing Parse.Error #7347

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

Open
wants to merge 7 commits into
base: alpha
Choose a base branch
from
Open

fix: server crashes on MongoDB GeoJSON error instead of throwing Parse.Error #7347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7b3493f
Forward Mongo geojson errors as Parse.Error.INVALID_VALUE
mstniy Apr 14, 2021
8c79194
Additional unit test for valid geometry
mstniy Apr 22, 2021
a9afea6
Merge branch 'master' into lean_7331
mstniy Apr 22, 2021
1804175
Depend on the latest js sdk release for the INVALID_VALUE error code
mstniy Aug 17, 2021
ed742aa
Merge remote-tracking branch 'official/master' into lean_7331
mstniy Aug 17, 2021
a9aad2e
Merge remote-tracking branch 'alpha' into HEAD
mstniy Mar 8, 2022
0301472
Also use INVALID_VALUE for object updates
mstniy Mar 8, 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
39 changes: 39 additions & 0 deletions spec/ParseAPI.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,45 @@ describe('miscellaneous', () => {
});
});

describe_only_db('mongo')('spatial index', () => {
beforeAll(async () => {
await TestUtils.destroyAllDataPermanently(false);
// Create a schema with a spatial index
const testSchema = new Parse.Schema('geojson_test');
testSchema.addObject('geometry');
testSchema.addIndex('geospatial_index', {
geometry: '2dsphere',
});
await testSchema.save();
Copy link
Member

Choose a reason for hiding this comment

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

I want to make sure we don't introduce a flaky test here. This awaits the schema save, but not the index building in DB.

Maybe you want to look into schema tests to see what do we usually use in these cases, a timeout?

cc @dplewis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test is not flaky for me. The collection starts empty, so index building should take close to no time.
I couldn't find any tests for addIndex on the server codebase. The JS sdk has several, but none of them seem to explicitly wait on index-building.

Copy link
Member

Choose a reason for hiding this comment

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

I think there is no way to verify index building via Parse, so the only way would be connecting directly to mongodb and checking the status. Or modify Parse adapter to include that. I believe this test will not be flaky but a timeout (maybe 1 or 2s) could be helpful as well.

Copy link
Member

Choose a reason for hiding this comment

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

Ok then let's merge it as it is and see how it goes. I will run the tests a few times here to see whether its flaky already.

});

it('invalid geometry fails (#7331)', async () => {
let obj = new Parse.Object('geojson_test');
obj.set('geometry', { foo: 'bar' });
try {
await obj.save();
fail('Invalid geometry did not fail');
} catch (e) {
expect(e.code).toEqual(Parse.Error.INVALID_VALUE);
}
obj = new Parse.Object('geojson_test');
await obj.save();
try {
obj.set('geometry', { foo: 'bar' });
await obj.save();
fail('Invalid geometry did not fail');
} catch (e) {
expect(e.code).toEqual(Parse.Error.INVALID_VALUE);
}
});

it('valid geometry succeeds', async () => {
const obj = new Parse.Object('geojson_test');
obj.set('geometry', { type: 'Point', coordinates: [-73.97, 40.77] });
await obj.save();
});
});

describe('miscellaneous', function () {
it('create a GameScore object', function (done) {
const obj = new Parse.Object('GameScore');
Expand Down
49 changes: 26 additions & 23 deletions src/Adapters/Storage/Mongo/MongoStorageAdapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,30 @@ function validateExplainValue(explain) {
}
}

function mongoErrorToParse(error) {
if (error.code === 11000) {
// Duplicate value
const err = new Parse.Error(
Parse.Error.DUPLICATE_VALUE,
'A duplicate value for a field with unique values was provided'
);
err.underlyingError = error;
if (error.message) {
const matches = error.message.match(/index:[\sa-zA-Z0-9_\-\.]+\$?([a-zA-Z_-]+)_1/);
if (matches && Array.isArray(matches)) {
err.userInfo = { duplicated_field: matches[1] };
}
}
return err;
} else if (error.code === 16755 || error.code === 16756) {
// Can't extract geo keys
const err = new Parse.Error(Parse.Error.INVALID_VALUE, error.message);
err.underlyingError = error;
return err;
}
return error;
}

export class MongoStorageAdapter implements StorageAdapter {
// Private
_uri: string;
Expand Down Expand Up @@ -481,22 +505,7 @@ export class MongoStorageAdapter implements StorageAdapter {
.then(collection => collection.insertOne(mongoObject, transactionalSession))
.then(() => ({ ops: [mongoObject] }))
.catch(error => {
if (error.code === 11000) {
// Duplicate value
const err = new Parse.Error(
Parse.Error.DUPLICATE_VALUE,
'A duplicate value for a field with unique values was provided'
);
err.underlyingError = error;
if (error.message) {
const matches = error.message.match(/index:[\sa-zA-Z0-9_\-\.]+\$?([a-zA-Z_-]+)_1/);
if (matches && Array.isArray(matches)) {
err.userInfo = { duplicated_field: matches[1] };
}
}
throw err;
}
throw error;
throw mongoErrorToParse(error);
})
.catch(err => this.handleError(err));
}
Expand Down Expand Up @@ -567,13 +576,7 @@ export class MongoStorageAdapter implements StorageAdapter {
)
.then(result => mongoObjectToParseObject(className, result.value, schema))
.catch(error => {
if (error.code === 11000) {
throw new Parse.Error(
Parse.Error.DUPLICATE_VALUE,
'A duplicate value for a field with unique values was provided'
);
}
throw error;
throw mongoErrorToParse(error);
})
.catch(err => this.handleError(err));
}
Expand Down