-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
feat(model): add create option "immediateError" #13544
Changes from 2 commits
b518fc4
1bb735f
84e6e0d
3f44286
f4d4105
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -479,7 +479,6 @@ function generateVersionError(doc, modifiedPaths) { | |
* newProduct === product; // true | ||
* | ||
* @param {Object} [options] options optional options | ||
* @param {Boolean} [options.ordered] saves the docs in series rather than parallel. | ||
* @param {Session} [options.session=null] the [session](https://www.mongodb.com/docs/manual/reference/server-sessions/) associated with this save operation. If not specified, defaults to the [document's associated session](https://mongoosejs.com/docs/api/document.html#Document.prototype.session()). | ||
* @param {Object} [options.safe] (DEPRECATED) overrides [schema's safe option](https://mongoosejs.com/docs/guide.html#safe). Use the `w` option instead. | ||
* @param {Boolean} [options.validateBeforeSave] set to false to save without validating. | ||
|
@@ -2802,6 +2801,8 @@ Model.findByIdAndRemove = function(id, options) { | |
* | ||
* @param {Array|Object} docs Documents to insert, as a spread or array | ||
* @param {Object} [options] Options passed down to `save()`. To specify `options`, `docs` **must** be an array, not a spread. See [Model.save](https://mongoosejs.com/docs/api/model.html#Model.prototype.save()) for available options. | ||
* @param {Boolean} [options.ordered] saves the docs in series rather than parallel. | ||
* @param {Boolean} [options.immediateError] If a Error occurs, throw the error immediately, instead of collecting in a result, default: true (backwards compatability) | ||
* @return {Promise} | ||
* @api public | ||
*/ | ||
|
@@ -2858,27 +2859,41 @@ Model.create = async function create(doc, options) { | |
return Array.isArray(doc) ? [] : null; | ||
} | ||
let res = []; | ||
const immediateError = typeof options.immediateError === 'boolean' ? options.immediateError : true; | ||
|
||
delete options.immediateError; // dont pass on the option to "$save" | ||
|
||
if (options.ordered) { | ||
for (let i = 0; i < args.length; i++) { | ||
const doc = args[i]; | ||
const Model = this.discriminators && doc[discriminatorKey] != null ? | ||
this.discriminators[doc[discriminatorKey]] || getDiscriminatorByValue(this.discriminators, doc[discriminatorKey]) : | ||
this; | ||
if (Model == null) { | ||
throw new MongooseError(`Discriminator "${doc[discriminatorKey]}" not ` + | ||
try { | ||
const doc = args[i]; | ||
const Model = this.discriminators && doc[discriminatorKey] != null ? | ||
this.discriminators[doc[discriminatorKey]] || getDiscriminatorByValue(this.discriminators, doc[discriminatorKey]) : | ||
this; | ||
if (Model == null) { | ||
throw new MongooseError(`Discriminator "${doc[discriminatorKey]}" not ` + | ||
`found for model "${this.modelName}"`); | ||
} | ||
let toSave = doc; | ||
if (!(toSave instanceof Model)) { | ||
toSave = new Model(toSave); | ||
} | ||
} | ||
let toSave = doc; | ||
if (!(toSave instanceof Model)) { | ||
toSave = new Model(toSave); | ||
} | ||
|
||
await toSave.$save(options); | ||
res.push(toSave); | ||
await toSave.$save(options); | ||
res.push(toSave); | ||
} catch (err) { | ||
if (!immediateError) { | ||
res.push(err); | ||
} else { | ||
throw err; | ||
} | ||
} | ||
} | ||
return res; | ||
} else { | ||
res = await Promise.all(args.map(async doc => { | ||
// ".bind(Promise)" is required, otherwise results in "TypeError: Promise.allSettled called on non-object" | ||
const promiseType = !immediateError ? Promise.allSettled.bind(Promise) : Promise.all.bind(Promise); | ||
let p = promiseType(args.map(async doc => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why can't this be just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i had avoided that because using |
||
const Model = this.discriminators && doc[discriminatorKey] != null ? | ||
this.discriminators[doc[discriminatorKey]] || getDiscriminatorByValue(this.discriminators, doc[discriminatorKey]) : | ||
this; | ||
|
@@ -2896,6 +2911,13 @@ Model.create = async function create(doc, options) { | |
|
||
return toSave; | ||
})); | ||
|
||
// chain the mapper, only if "allSettled" is used | ||
if (!immediateError) { | ||
p = p.then(presult => presult.map(v => v.status === 'fulfilled' ? v.value : v.reason)); | ||
} | ||
|
||
res = await p; | ||
} | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,7 +123,6 @@ declare module 'mongoose' { | |
SessionOption { | ||
checkKeys?: boolean; | ||
j?: boolean; | ||
ordered?: boolean; | ||
safe?: boolean | WriteConcern; | ||
timestamps?: boolean | QueryTimestampsConfig; | ||
validateBeforeSave?: boolean; | ||
|
@@ -132,6 +131,11 @@ declare module 'mongoose' { | |
wtimeout?: number; | ||
} | ||
|
||
interface CreateOptions extends SaveOptions { | ||
ordered?: boolean; | ||
immediateError?: boolean; | ||
} | ||
|
||
interface RemoveOptions extends SessionOption, Omit<mongodb.DeleteOptions, 'session'> {} | ||
|
||
const Model: Model<any>; | ||
|
@@ -217,7 +221,7 @@ declare module 'mongoose' { | |
>; | ||
|
||
/** Creates a new document or documents */ | ||
create<DocContents = AnyKeys<TRawDocType>>(docs: Array<TRawDocType | DocContents>, options?: SaveOptions): Promise<THydratedDocumentType[]>; | ||
create<DocContents = AnyKeys<TRawDocType>>(docs: Array<TRawDocType | DocContents>, options?: CreateOptions): Promise<THydratedDocumentType[]>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to add TypeScript types to indicate that, if |
||
create<DocContents = AnyKeys<TRawDocType>>(doc: DocContents | TRawDocType): Promise<THydratedDocumentType>; | ||
create<DocContents = AnyKeys<TRawDocType>>(...docs: Array<TRawDocType | DocContents>): Promise<THydratedDocumentType[]>; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I like this option name. I was thinking
continueOnError
to be consistent witheachAsync()
andcreateCollections()
, butcontinueOnError
aggregates an error object to throw when an error occurs. Do you have any other naming ideas? MaybeuseAllSettled
oraggregateErrors
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i had named it
immediateError
because of the current (master) behavior where once a error is encountered the first one is directly thrown / rejected, but all operations should still run and any subsequent error is just silentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we name the option
aggregateErrors
instead? I think that options that are treated asfalse
by default are cleaner to work with.