Don't call onError() with internal exception (#2603)

lib/queryBuilder/InternalOptions.js

@@ -4,6 +4,7 @@ class InternalOptions {
  constructor() {
  this.skipUndefined = false;
  this.keepImplicitJoinProps = false;
+ this.returnImmediatelyValue = undefined;
  this.isInternalQuery = false;
  this.debug = false;
  this.schema = undefined;
@@ -14,6 +15,7 @@ class InternalOptions {
 
  copy.skipUndefined = this.skipUndefined;
  copy.keepImplicitJoinProps = this.keepImplicitJoinProps;
+ copy.returnImmediatelyValue = this.returnImmediatelyValue;
  copy.isInternalQuery = this.isInternalQuery;
  copy.debug = this.debug;
  copy.schema = this.schema;

lib/queryBuilder/QueryBuilder.js

@@ -1119,7 +1119,24 @@ function afterExecute(builder, result) {
  return promise;
 }
 
+class ReturnImmediatelyException {
+ constructor(value) {
+ this.value = value;
+ }
+}
+
+function handleReturnImmediatelyValue(builder) {
+ const { returnImmediatelyValue } = builder.internalOptions();
+ if (returnImmediatelyValue !== undefined) {
+ throw new ReturnImmediatelyException(returnImmediatelyValue);
+ }
+}
+
 function handleExecuteError(builder, err) {
+ if (err instanceof ReturnImmediatelyException) {
+ return Promise.resolve(err.value);
+ }
+
  let promise = Promise.reject(wrapError(err));
 
  builder.forEachOperation(true, (op) => {
@@ -1139,9 +1156,11 @@ function chainOperationHooks(promise, builder, hookName) {
 
  builder.forEachOperation(true, (op) => {
  if (op.hasHook(hookName)) {
- promise = promise.then((result) =>
- builder.callAsyncOperationMethod(op, hookName, [builder, result]),
- );
+ promise = promise.then((result) => {
+ const res = builder.callAsyncOperationMethod(op, hookName, [builder, result]);
+ handleReturnImmediatelyValue(builder);
+ return res;
+ });
  }
  });
 
@@ -1261,7 +1280,11 @@ async function chainHooks(promise, builder, func) {
  let promise = Promise.resolve(result);
 
  if (isFunction(func)) {
- promise = promise.then((result) => func.call(builder, result, builder));
+ promise = promise.then((result) => {
+ const res = func.call(builder, result, builder);
+ handleReturnImmediatelyValue(builder);
+ return res;
+ });
  } else if (Array.isArray(func)) {
  func.forEach((func) => {
  promise = chainHooks(promise, builder, func);

lib/queryBuilder/graph/patch/GraphPatchAction.js

@@ -82,24 +82,15 @@ class GraphPatchAction extends GraphAction {
  .copyFrom(builder, GraphAction.ReturningAllSelector);
 
  if (hasBeforeUpdate) {
- updateBuilder
- .context({
- runBefore: () => {
- // Call handleUpdate in the runBefore hook which runs after the
- // $beforeUpdate hook, allowing it to modify the object before the
- // updated properties are determined. See issue #2233.
- if (hasBeforeUpdate && !handleUpdate()) {
- throw new ReturnNullException();
- }
- },
- })
- .onError((error) => {
- if (error instanceof ReturnNullException) {
- return null;
- } else {
- return Promise.reject(error);
- }
- });
+ updateBuilder.internalContext().runBefore.push((result, builder) => {
+ // Call handleUpdate in the runBefore hook which runs after the
+ // $beforeUpdate hook, allowing it to modify the object before the
+ // updated properties are determined. See issue #2233.
+ if (hasBeforeUpdate && !handleUpdate()) {
+ builder.internalOptions().returnImmediatelyValue = null;
+ }
+ return result;
+ });
  }
 
  if (shouldPatch) {
@@ -235,8 +226,6 @@ class GraphPatchAction extends GraphAction {
  }
 }
 
-class ReturnNullException {}
-
 function childQueryOptions() {
  return {
  fork: true,

tests/integration/upsertGraph.js

@@ -4131,6 +4131,34 @@ module.exports = (session) => {
  });
  });
 
+ describe('should not call onError() with internal exception (#2603)', () => {
+ let query;
+
+ before(() => {
+ query = Model1.query;
+ });
+
+ after(() => {
+ Model1.query = query;
+ });
+
+ it('should not call onError() with internal exception', async () => {
+ const upsert = { id: 2 };
+
+ let error = null;
+ Model1.query = function (trx) {
+ return query.call(this, trx).onError((err) => {
+ error = err;
+ });
+ };
+
+ await transaction(session.knex, (trx) =>
+ Model1.query(trx).upsertGraph(upsert, { fetchStrategy }),
+ );
+ expect(error).to.equal(null);
+ });
+ });
+
  if (session.isPostgres()) {
  describe('returning', () => {
  it('should propagate returning(*) to all update an insert operations', () => {