Mikro-orm: ValidationError: You cannot call em.flush() from inside lifecycle hook handlers

[asmimo]

Describe the bug
My problem is related to #627 . As pretty much same thing is happening to me but not a custom repository but a service class. Is it cause I am not using a DI? I don't even know.
I first used apollo-server-fastify and thought it had to something to do with fastify so I switched to apollo-server-express but it didn't solve the issue. I tried adding RequestContext middleware but it still didn't work.

Here is the github repo. It contains a single user entity with basic crud operations. But here are the main files;

1. Server init

import express from "express";
import { ApolloServer } from "apollo-server-express";
import { MikroORM, RequestContext } from "mikro-orm";
import config from "./mikro-orm.config";
import { buildSchema } from "type-graphql";

export const DI = {} as {
  orm: MikroORM;
};
// Init Server
(async () => {
  const app = express();
  const port = process.env.PORT ? Number(process.env.PORT) : 3010;

  try {
    DI.orm = await MikroORM.init(config);

    const apolloServer = new ApolloServer({
      schema: await buildSchema({
        resolvers: [__dirname + "/modules/**/*.resolver.{ts,js}"],
        emitSchemaFile: false,
      }),
      context: ({ req, res }) => ({ req, res }),
    });

    app.use((_req, _res, next) => {
      RequestContext.create(DI.orm.em, next);
    });

    apolloServer.applyMiddleware({ app });

    app.listen(port, "0.0.0.0", () => console.log(`Server running on port: ${port}`));
  } catch (err) {
    console.log(err);
    process.exit(1);
  }
})();

1. Orm config

import { Options } from "mikro-orm";
import { BaseEntity } from "./globals/entity";
import { User } from "./modules/user/user.entity";

const config: Options = {
  type: "postgresql",
  clientUrl: "postgres://postgres:test@localhost:5432/mikro",
  entities: [BaseEntity, User],
  migrations: {
    path: "./src/migrations",
  },
};

export default config;

1. User Entity

import { Entity, Property } from "mikro-orm";
import { BaseEntity } from "../../globals/entity";
import { ObjectType, Field, Int } from "type-graphql";
import bcrypt from "bcryptjs";

@ObjectType()
@Entity()
export class User extends BaseEntity {
  @Field()
  @Property({ unique: true })
  username: string;

  @Field()
  @Property({ unique: true })
  email: string;

  @Property()
  password: string;

  @Field(() => Int)
  @Property()
  age: number;

  @Field()
  @Property()
  sex: string;

  async hashPassword(): Promise<void> {
    const salt = await bcrypt.genSalt();
    this.password = await bcrypt.hash(this.password, salt);
  }

  async comparePassword(password: string): Promise<boolean> {
    return bcrypt.compare(password, this.password);
  }
}

1. User Resolver

import { Resolver, Mutation, Query, Args, Arg } from "type-graphql";
import { User } from "./user.entity";
import UserService from "./user.service";
import {
  CreateUserDTO,
  UserFilterDTO,
  UserPaginationDTO,
  IdDTO,
  UpdateUserDTO,
  UpdateUserPasswordDTO,
} from "./user.dto";

@Resolver(User)
export class UserResolver {
  public readonly userService = new UserService();

  @Mutation(() => User)
  async createUser(@Args() dto: CreateUserDTO): Promise<User> {
    return this.userService.createUser(dto);
  }

  @Query(() => [User])
  async getUsers(
    @Arg("filter", { nullable: true }) filter: UserFilterDTO,
    @Args() pagination: UserPaginationDTO,
  ): Promise<User[]> {
    return this.userService.getUsers(filter, pagination);
  }

  @Query(() => User)
  async getUser(@Args() { id }: IdDTO): Promise<User> {
    return this.userService.getUser(id);
  }

  @Mutation(() => User)
  async updateUser(@Args() { id }: IdDTO, @Args() dto: UpdateUserDTO): Promise<User> {
    return this.userService.updateUser(id, dto);
  }

  @Mutation(() => User)
  async updateUserPassword(@Args() { id }: IdDTO, @Args() dto: UpdateUserPasswordDTO): Promise<User> {
    return this.userService.updateUserPassword(id, dto);
  }

  @Mutation(() => Boolean)
  async deleteUser(@Args() { id }: IdDTO): Promise<boolean> {
    return this.userService.deleteUser(id);
  }
}

1. User Service

export default class UserService {
  protected readonly userRepo = DI.orm.em.getRepository(User);

  async createUser(dto: CreateUserDTO): Promise<User> {
    const { username, email, password, confirm, age, sex } = dto;

    if (password !== confirm) {
      throw new Error("match fail");
    }

    const user = this.userRepo.create({
      username,
      email,
      password,
      age,
      sex,
    });
    try {
      await user.hashPassword();
      await this.userRepo.persist(user, true);
    } catch (error) {
      if (error.code === "23505") {
        const rxp = /\(([^)]+)\)/;
        const key = rxp.exec(error.detail);
        if (key) {
          throw new Error(`${key[1]} already exists`);
        }
      } else {
        throw new Error(error);
      }
    }

    return user;
  }

async getUsers(filter: UserFilterDTO, pagination: UserPaginationDTO): Promise<User[]> {
    const query = this.userRepo.createQueryBuilder("user");

    if (filter) {
      const { username, email } = filter;
      if (username != null) {
        query.where({ username: { $like: username } });
      }
      if (email != null) {
        // This is how I wrote in TYPEORM
        // query.andWhere("user.email ILIKE :email", {email});
        // what is the equivalent of this in mikro-orm
      }
    }

    const { limit, offset, sort, by } = pagination;
    query
      .limit(limit)
      .offset(offset)
      .orderBy({ [by]: sort });

    console.log(query.getQuery(), query.getParams());

    return query.getResult();
  }
}

Can you review the code too and see if I can improve it or something that I did bad?
Also how do you use ILIKE

Stack trace

ValidationError: You cannot call em.flush() from inside lifecycle hook handlers
    at Function.cannotCommit (/home/red/Documents/work/rewise/server/node_modules/mikro-orm/dist/utils/ValidationError.js:129:16)
    at UnitOfWork.commit (/home/red/Documents/work/rewise/server/node_modules/mikro-orm/dist/unit-of-work/UnitOfWork.js:87:43)
    at EntityManager.flush (/home/red/Documents/work/rewise/server/node_modules/mikro-orm/dist/EntityManager.js:330:36)
    at EntityManager.persistAndFlush (/home/red/Documents/work/rewise/server/node_modules/mikro-orm/dist/EntityManager.js:275:20)
    at EntityManager.persist (/home/red/Documents/work/rewise/server/node_modules/mikro-orm/dist/EntityManager.js:265:25)
    at EntityRepository.persist (/home/red/Documents/work/rewise/server/node_modules/mikro-orm/dist/entity/EntityRepository.js:9:24)
    at UserService.<anonymous> (/home/red/Documents/work/rewise/server/src/modules/user/user.service.ts:24:27)
    at Generator.next (<anonymous>)
    at fulfilled (/home/red/Documents/work/rewise/server/src/modules/user/user.service.ts:5:58) {
  entity: undefined
}

To Reproduce
Steps to reproduce the behavior:

1. Create a user
2. Create/update another user with already used email/username
   Now the method is unusable

Expected behavior
Expected to work normal but after unique constraint error is thrown, following request on that method fails showing this error.

Additional context
...

Versions

| Dependency | Version |
| - | - |
| node | 12.18.0 |
| typescript | 3.9.7 |
| mikro-orm | 3.6.15 |

[B4nan]

I am not a GQL user, so can't help much with this, check out this for an inspiration #364, namely this bit:

https://github.com/mikro-orm/mikro-orm/pull/364/files#diff-55ef89a7f4c6088e8823c512cad7d84eR56

And yes, you definitely use the same context instead of the fork if this happens to you. Maybe it would be enough to register it sooner.

Also how do you use ILIKE

In v4 there is $ilike operator available.

Btw its nice that you provided a repro, but please also give me a way to really reproduce, e.g. give me curl requests I should execute. Ideally this should be done as a testcase/standalone script.

[asmimo]

Oh sorry didn't thought of that. Here is the request if you want to replicate:

1. Open localhost:3010/graphql
2. Copy and paste

mutation CreateUser {
  createUser(
    username: "testuser"
    email: "[email protected]"
    password: "Test0ne"
    confirm: "Test0ne"
    age: 23
    sex: MALE
  ) {
    id
    username
    email
    age
    sex
  }
}

or if you want in curl

curl 'http://localhost:3010/graphql' -H 'Accept-Encoding: gzip, deflate, br' -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'Connection: keep-alive' -H 'DNT: 1' -H 'Origin: http://localhost:3010' -H 'authorization: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImVjNjg5NGExLTRhYzMtNGFiNy1hMzQxLTE5Njg3N2U1NzBjZSIsInR3b19zdGVwIjpmYWxzZSwidHdvX3N0ZXBfa2V5IjpudWxsLCJpYXQiOjE1ODE4NTc0NDgsImV4cCI6MTU4MTk4NzA0OH0._Ee7OSREI4GWWzW4EZlHFAQFXRLEYyA-L_wCbDl0Cu0' --data-binary '{"query":"mutation CreateUser {\n  createUser(\n    username: \"testuser\"\n    email: \"[email protected]\"\n    password: \"Test0ne\"\n    confirm: \"Test0ne\"\n    age: 23\n    sex: MALE\n  ) {\n    id\n    username\n    email\n    age\n    sex\n  }\n}"}' --compressed

1. Run it twice
   And from that onward call to this method fails.

Gonna try fork for now and see if it works.
Thanks for the hard work.

[B4nan]

when I execute that twice, I get unique constrain violation, which makes sense to me

[asmimo]

do it more than two time

[B4nan]

yeah when executed for the third time, i see that ValidationError error about concurrent flushing. after restarting the server, it behaves like this again, first hit does return the unique error, second the concurrent flush error, so i am sure this is caused by using the global context.

[asmimo]

yeah I got it working using fork. Basically provide new forked repository through middleware. Here's the code:

server.ts

import express from "express";
import { ApolloServer } from "apollo-server-express";
import { MikroORM, EntityRepository } from "mikro-orm";
import config from "./mikro-orm.config";
import { buildSchema } from "type-graphql";
import { User } from "./modules/user/user.entity";

export const DI = {} as {
  orm: MikroORM;
  userRepo: EntityRepository<User>
};
// Init Server
(async () => {
  const app = express();
  const port = process.env.PORT ? Number(process.env.PORT) : 3010;

  try {
    DI.orm = await MikroORM.init(config);

    const apolloServer = new ApolloServer({
      schema: await buildSchema({
        resolvers: [__dirname + "/modules/**/*.resolver.{ts,js}"],
        emitSchemaFile: false,
      }),
      context: ({ req, res }) => ({ req, res }),
    });

    app.use((_req, _res, next) => {
      // here's the magic code
      DI.userRepo = DI.orm.em.fork().getRepository(User)
      next()
    });

    apolloServer.applyMiddleware({ app });

    app.listen(port, "0.0.0.0", () => console.log(`Server running on port: ${port}`));
  } catch (err) {
    console.log(err);
    process.exit(1);
  }
})();

user.service.ts

import { DI } from "../../server";
import { User } from "./user.entity";
import { CreateUserDTO, UserFilterDTO, UserPaginationDTO, UpdateUserDTO, UpdateUserPasswordDTO } from "./user.dto";

export default class UserService {
  // -- protected readonly userRepo = DI.orm.em.getRepository(User);
  async createUser(dto: CreateUserDTO): Promise<User> {
    const { username, email, password, confirm, age, sex } = dto;

    if (password !== confirm) {
      throw new Error("match fail");
    }

    const user = DI.userRepo.create({
      username,
      email,
      password,
      age,
      sex,
    });
    try {
      await user.hashPassword();
      await DI.userRepo.persist(user, true);
    } catch (error) {
      if (error.code === "23505") {
        const rxp = /\(([^)]+)\)/;
        const key = rxp.exec(error.detail);
        if (key) {
          throw new Error(`${key[1]} already exists`);
        }
      } else {
        throw new Error(error);
      }
    }
}

And with a little bit of refactor I can clean up server file and shove all mikro-orm code in its config file. Thanks @B4nan .

[B4nan]

Apparently the RequestContext helper does not work, which means the apollo middleware breaks it somehow. Checking the EM id now, and I can see that you get the global one in your user service.

[info] MikroORM successfully connected to database test.db
Server running on port: 3010
creating new context b7fa286f-3aeb-4769-8800-cfb126a1bf15
using context from service d9d8b6d8-c5d0-4191-9353-4031b08d8701
[query] begin
[query] insert into `user` (`age`, `created_at`, `email`, `id`, `password`, `sex`, `updated_at`, `username`) values (23, '2020-07-31 15:46:45.361', '[email protected]', '1a4b7a74-a037-49c4-afb1-8334d056ee5b', '$2a$10$VWykIaLY2vPQIZjpJCVAJujLNaG0b/SJZ.XVo5.0lvxhGLrZ9XZLa', 'MALE', '2020-07-31 15:46:45.361', 'testuser')39m [took 1 ms]
[query] rollback
creating new context 7beddd9c-ed43-47c6-88a6-0f0bd540dc4c
using context from service d9d8b6d8-c5d0-4191-9353-4031b08d8701

I am not sure what happens there, the context gets created fine, but apollo is probably executing the handled in a different way, that breaks the domain API (which is used in RequestContext helper).

So you will probably need to use manual forking, which is how the mentioned PR does it too.

yeah I got it working using fork. Basically provide new forked repository through middleware. Here's the code:

I dont think you got it right. Afaik you can't rely on switching single global property, as (async) request handlers can be executed simultaneously. You should pass the fork together with the req object, as that is basically the request context.

[asmimo]

How do i use ILIKE in v3. Well I am going to start a new project. Do you recommend using v4 or should i wait a little?

[B4nan]

I would go for v4, latest alpha is pretty much the last one before RC (I might ship one more with some small fixes connected to the type safe references). I am using that in my production app without any issues.

In v3 you could do this with query builder:

qb.select('*').where('comment ilike ?', ['...']);

// you can also do this in some nested condition
qb.select('*').where({
  name: '...',
  age: { $gte: 18 },
  'comment ilike ?': ['...'],
});

The second approach should work with EM too, but it won't type check, so you would have to cast as any or something:

em.find(User, {
  age: { $gte: 18 },
  'name ilike ?': ['...'],
} as any);

Btw in v4 the db errors are wrapped, so you don't have to parse error messages to look for the error codes like this, you can just check the instance of the error, as you will get an instance of UniqueConstraintViolationException.

[asmimo]

well I upgraded the project to v4 but here are the changes and errors:

• persist/remove(entity, true) -> persist/remove(entity).flush(). [fixed]
• Property 'createQueryBuilder' does not exist on type 'EntityRepository'. [couldn't fix]
• had this for delete operation [couldn't fix]

// result is void previously it provided 0 or 1.
const result = await this.userRepo.remove({ id }).flush();
return result ? true : false;

[B4nan]

read the migration guide, you need to import the repository from driver package and remove now works only with entities, use nativeDelete if you want to fire a delete query based on PK, or em.getReference()

[B4nan]

Something like this should work for the forking:

// adjust the Request type
declare global {
  namespace Express {
    export interface Request {
      em?: EntityManager;
    }
  }
}

app.use((req, _res, next) => {
  req.em = DI.orm.em.fork();
  next();
});

and then instead of the global DI object, get the EM from req.em (idk how to do that - never used apollo - but should be doable, right?)

Btw the migration guide for v4 is here: https://github.com/mikro-orm/mikro-orm/blob/dev/docs/docs/upgrading-v3-to-v4.md

[B4nan]

I guess you should be somehow able to obtain the context you specify in the ApolloServer ctor, if so, it would be best to pass the forked EM there:

const apolloServer = new ApolloServer({
  schema: await buildSchema({
    resolvers: [__dirname + "/modules/**/*.resolver.{ts,js}"],
    emitSchemaFile: false,
  }),
  context: ({ req, res }) => {
    const em = DI.orm.em.fork();
    return { req, res, em };
  },
});

(will be happy to hear how to do that, was trying to google it for past few minutes without luck :D)

[asmimo]

Sorry for the late reply, I was stuck on the declare global for solid till now and when I refreshed saw the context one and I like the context one better. there will be lot more repeated code to write but I imagine the code will work. There will be a lot of boilerplate but one way to reduce it is using custom repo instead of service and will look on other ways. Will update you the progress when its done.

[asmimo]

Do we not get type definiton by doing this:

author.repository.ts

export class CustomAuthorRepository extends EntityRepository<Author> {
  // your custom methods...
  public findAndUpdate(...) {
    // ...
  }
}

author.entity.ts

@Entity({ customRepository: () => CustomAuthorRepository })
export class Author {
  // ...
}

And using it this way

const authorRepo = em.getRepository(Author);
// @ts-ignore
authorRepo.findAndUpdate(...)

Custom method is definitely working but we have to ignore type checking.

[B4nan]

There is now way TS would know what the repository type is currently, you need to type cast. But I was recently thinking about making this at least configurable from the entity definition - all we need is a property that will hold the type of the reference:

@Entity({ customRepository: () => CustomAuthorRepository })
export class Author {

  [EntityRepositoryType]?: CustomAuthorRepository;

}

This will be part of next v4 release.

[B4nan]

I will close this as we found the root cause, I don't think there is more I can help with here.

[matthieu-architectoile]

Hello,

I am experiencing the same issue "ValidationError: You cannot call em.flush() from inside lifecycle hook handlers" when flushing a second time after the first one crashed on unique constraint.

em.persist(myEntity);
try {
  em.flush(); // Fails because of unique constraint violation
}
catch(e) {
  if (e instanceof DatabaseError && e.code === '23505') {
    // Handle duplicate error
  }
}
// Few lines later
em.persist(anotherEntity); // Those entity's data are correct
em.flush(); // Throws : ValidationError: You cannot call em.flush() from inside lifecycle hook handlers

The exception thrown is _DatabaseError_ from the node.js _pg_ module (used by MikroORM):

error: insert into "XXX" ("XXX", "XXX", ...) values ($1, $2, $3, $4, $5) returning "XXX" - duplicate key value violates unique constraint "uniq_YYY"

at Parser.parseErrorMessage (../node_modules/pg-protocol/src/parser.ts:357:11)
at Parser.handlePacket (../node_modules/pg-protocol/src/parser.ts:186:21)
at Parser.parse (../node_modules/pg-protocol/src/parser.ts:101:30)
at Socket.stream.on (../node_modules/pg-protocol/src/index.ts:7:48)

My guess is the ORM rollbacks correctly but since the exception is not its own, it does not come back to an idle/available state.

Doing a dirty em.unitOfWork.working = false; hacked the issue:

em.persist(myEntity);
try {
  em.flush(); // Fails because of unique constraint violation
}
catch(e) {
  if (e instanceof DatabaseError && e.code === '23505') {
    // Handle duplicate error
    em.unitOfWork.working = false;
  }
}
// Few lines later
em.persist(anotherEntity); // Those entity's data are correct
em.flush(); // Now working! :D

The ORM is now available for new tasks. :-)

I am using version MikroORM version 3.6.15.

[B4nan]

You need to have dedicated contexts for each request. Setting the working flag manually might help to shut up the validation, but it wont solve the problem itself. Dont do that, it will burn you later in production, when concurrent requests will be comming.

Edit: ok now I get it, I would suggest to create new EM fork instead, the UoW state could be invalid after an exception is thrown.

[matthieu-architectoile]

Thanks for the quick answer!

My whole quoted code belong to a same client/request.

Changing manually the "working" flag is a dirty fix as I wrote. I intend to leave this only until this issue is fixed.
Note that the flag change is made only for this specific exception catch (allowing the UoW state to stay invalid for other exceptions).
Since you suggest to drop the entity manager instead, I'll comply. Noticing it is still a dirty fix to drop the whole entity manager and instantiate a new one.

My point was to say "The unit of work should not break after a constraint violation." and to give a clue where the issue may happen. Sorry if I haven't been explicit enough.

[asmimo]

i will update my repo of the working code and will leave it there for any future references. Anyone getting the same issue can look up the code.

Edit: Here's the repo. It uses apollo-server-express with type-graphql.

[B4nan]

Changing manually the "working" flag is a dirty fix as I wrote. I intend to leave this only until this issue is fixed.
Note that the flag change is made only for this specific exception catch (allowing the UoW state to stay invalid for other exceptions).
Since you suggest to drop the entity manager instead, I'll comply. Noticing it is still a dirty fix to drop the whole entity manager and instantiate a new one.

My point was to say "The unit of work should not break after a constraint violation." and to give a clue where the issue may happen. Sorry if I haven't been explicit enough.

There won't be any fix for this. As I said, you should use clean EM (so creating new fork or calling em.clear()) after an error is thrown. In this simple example, it might look like its a good idea to reuse the UoW, but in complex apps this won't fly. There might be many changes in the UoW, one of them causes the error, but all the entities can be interconnected, and persisting "another one that won't produce the error" is not enough, as you still have all the old entities in it. Do this instead:

try {
  em.persist(myEntity);
  await em.flush(); // Fails because of unique constraint violation
} catch(e) {
  // ...
}
// Few lines later
em.clear(); // reset the EM (and its UoW)
// em = em.fork(); // or create new fork
em.persist(anotherEntity); // Those entity's data are correct
em.flush(); // Now working! :D

Or you can use em.clear() which is pretty much the same as creating new fork (it will also reset the UoW).

[asmimo]

Hey @B4nan , can u look at the repo i updated the repo to use alpha 12 for #698 but all the query fails.

1. Open localhost:3010/graphql
2. Copy and paste

mutation CreateUser {
  createUser(
    username: "testuser"
    email: "[email protected]"
    password: "Test0ne"
    confirm: "Test0ne"
    age: 23
    sex: MALE
  ) {
    id
    username
    email
    age
    sex
  }
}

query GetUser {
  getUser(id: "5231c522-ba2d-41c6-ba1c-960930632c2c") {
    id
    username
    email
    age
    sex
  }
}


mutation DeleteUser {
  deleteUser(id: "5231c522-ba2d-41c6-ba1c-960930632c2c")
}

1. Put your cursor on one request and click play
   Every request gives this error message at the end:
   [createUser has 'DriverException:']...queries... invalid input syntax for type json
   Or should i create a new issue for this.

[B4nan]

entities: [BaseEntity, "src/modules/**/*.entity.{ts,js}", "dist/modules/**/*.entity.{ts,js}"],

This is definitely wrong. entities are for compiled JS files, entitiesTs are for the TS source files. You can't mix it.

You should do this instead:

entities: [BaseEntity, "dist/modules/**/*.entity.js"],
entitiesTs: [BaseEntity, "src/modules/**/*.entity.ts"],

Anyway, tried it and it works fine for me, no errors (even without this change - but that is probably because I do not have dist folder at all as I did not compile anything and used yarn dev to run the project via ts-node). You probably have badly defined schema, as I created one from the metadata (also used sqlite so I don't have to configure anything). The error comes from the database, it has nothing to do with using EntityRepositoryType symbol.

[B4nan]

Looking at your entity definition, I don't see anything that should be mapped to JSON column (and the error is caused by you having some JSON column, but providing invalid JSON value for it).

Maybe it is caused by the createdAt and updatedAt fields, in v4 the default metadata provider is ReflectMetadataProvider, where you need to define the type explicitly, even if TS can infer it just fine, but without the explicit declaration it won't be available in the reflect-metadata.

@ObjectType()
export abstract class BaseEntity {
  @Field(() => ID)
  @PrimaryKey()
  id: string = v4();

  @Field(() => Date)
  @Property()
  createdAt: Date = new Date();

  @Field(() => Date)
  @Property({ onUpdate: () => new Date() })
  updatedAt: Date = new Date();
}

And be sure to fix your schema, without it it will never work. You should see what column is causing the error (unfortunately you truncated the error message so I don't have a clue).

Or use TsMorphMetadataProvider:

MikroORM.init({
  // ...
  metadataProvider: TsMorphMetadataProvider,
});

[asmimo]

Thanks man. You are awesome.

[xeladejo]

The trouble with apollo-server-express seems is because it include and enable by default body-parser.

if disabling body-parser in apollo-server-express and handle it manually before RequestContext all seems work well:

application.use("/", bodyParser.json());
application.use((_req, _res, next) => RequestContext.create(DI.orm.em, next));
apolloServer.applyMiddleware({ app: application, path: "/", bodyParserConfig: false });