Skip to content
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

MigrationError: Migration 2024.06.20T07.13.41.random_file_name.ts (up) failed: Original error: migration.up is not a function #693

Open
Yash17Agrawal opened this issue Sep 30, 2024 · 8 comments

Comments

@Yash17Agrawal
Copy link

"mysql2": "^3.10.0",
"sequelize": "^6.37.3",
"umzug": "^3.8.1"
node version v20.10.0
We have migrations written in typescript like

import { MigrationParams } from "@local-types";
import { DataTypes } from "sequelize";

export const up = async ({ context: sequelize }: MigrationParams) => {
    await sequelize.addColumn(
       // some code
    );
};

export const down = async ({ context: sequelize }: MigrationParams) => {
    await sequelize.removeColumn(
        // some code
    );
};

which gets compiled to .js files and is run through nodejs lambda

The error is happening randomly without any concurrent pattern seen.
FYI: we have few files like random_file.ts.ts in between as tech debt but couldnt reproduce with them as well

also this is how i am initializing umzug

import fs from "fs";
import path from "path";
import { Sequelize } from "sequelize";
import { Umzug, SequelizeStorage } from "umzug";

interface Props {
    databaseName: string;
}

export const getUmzug = (
    sequelize: Sequelize,
    props?: Props,
    migrationsGlob?: string
) => {
    return new Umzug({
        migrations: {
            glob: "build/migrations/*.{ts,js}",
            resolve: ({ name, path, context }) => {
                const migration = require(path);
                return {
                    name: name.replace(/\.js$/, ".ts"),
                    up: async () =>
                        migration.up({
                            context,
                            ...(props && { databaseName: props.databaseName })
                        }),
                    down: async () => migration.down({ context })
                };
            }
        },
        context: sequelize.getQueryInterface(),
        storage: new SequelizeStorage({
            sequelize: sequelize,
            tableName: process.env.DB_META_TABLE_NAME || "SequelizeMeta"
        }),
        logger: console,
        create: {
            folder: "src/migrations",
            template: filePath => [
                [
                    filePath,
                    fs.readFileSync(
                        path.join(__dirname, "template", "sample-migration.ts"),
                        "utf-8"
                    )
                ]
            ]
        }
    });
};
@pharapeti
Copy link

Same issue here. I believe this issue has been introduced after we moved from CJS (module: commonjs) to NodeNext (module: NodeNext).

Does anybody have a solution for this?

@mmkal
Copy link
Contributor

mmkal commented Jan 8, 2025

Worth trying going from const migration = require(path) to const migration = await import(path) (after making the up function async). Might be a named exports->require oddity.

If that doesn't work - if you can create a reproduction on stackblitz, I can take a look.

@MyNameIsNeXTSTEP
Copy link

MyNameIsNeXTSTEP commented Feb 27, 2025

@mmkal Trying await import(path) for the ES modules doesn't help unfortunetely.

I tried this package aorund for the first time, but stumbled upon this issue for half of a day.

See my simple example:

app/migrations/20250228.migration.js

import { DataTypes } from '@sequelize/core';

export default {
  async up({ context: queryInterface }) {
    await queryInterface.createTable('Users', {
      id: {
        type: DataTypes.INTEGER,
        autoIncrement: true,
        primaryKey: true,
      },
      balance: {
        type: DataTypes.FLOAT,
        defaultValue: 0,
        allowNull: false,
      },
      createdAt: DataTypes.DATE,
      updatedAt: DataTypes.DATE,
    });
  },

  async down({ context }) {
    await context.dropTable('Users');
  },
};

app/migrations/umzug.js

import { Sequelize } from 'sequelize';
import { Umzug, SequelizeStorage } from 'umzug';
import connection from '../config/connection.js';

import { resolve, dirname  } from 'path';
import { fileURLToPath } from 'url';

const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);

const { database, username, password, host, port, dialect } = connection.development;

const sequelize = new Sequelize(
  database,
  username,
  password,
  {
    host,
    port,
    dialect,
  }
);

const umzug = new Umzug({
  migrations: {
    glob: 'app/migrations/*.migration.js',
    resolve: async ({ name, path, context }) => {
      const migration = await import(resolve(__dirname, path));
      return {
        name,
        up: async () => await migration.up({ context }),
        down: async () => await migration.down({ context }),
      };
    },
  },
  context: sequelize.getQueryInterface(),
  storage: new SequelizeStorage({ sequelize }),
  logger: console,
});

export default umzug;

app/migrations/runMigrations.js

import umzup from './umzug.js';

(async () => {
  try {
    console.log('Started migrations');
    await umzug.up();
    console.log('Migrated successefuly');
    return;
  } catch (error) {
    console.error('Error when tried to mirate:', error);
  }
})();

And finally the error:

{ event: 'migrating', name: undefined }
Error when tried to mirate: MigrationError: Migration undefined (up) failed: Original error: m.up is not a function
    at /app/node_modules/umzug/lib/umzug.js:170:27
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async Umzug.runCommand (/app/node_modules/umzug/lib/umzug.js:126:20)
    at async file:///app/app/migrations/runMigrations.js:6:5 {
  cause: TypeError: m.up is not a function

Even though when i logged the migration.up function after the await import it was actually the [AsyncFunction: up], meaning that the migration script functions, e.g the up, were actually imported correctly, and any issue was fired after.

I don't know yet where and why...

@MyNameIsNeXTSTEP
Copy link

MyNameIsNeXTSTEP commented Feb 27, 2025

After a bit of playing around with the source code i've got another issue here.

In the source of umzug/lib/umzug.js there's code:

Image

Here the green blocks is what i've added to test, the red one was commented.
So here clearly the source code in ES modules when we trying to call the .up() method on a migration gets not a migration at all, but the object in a form of:

{ path: 'path-to-migration-script' }

So i made it load the actual script as you might see on the picture, and here i got the actual migration script .up() function in ESM like [AsyncFunction: up], but... :)

Even after that - another error has occured:

[AsyncFunction: up] // the ESM `up` function
THE M UP FUNCTION // my log

Executing (default): SELECT table_name FROM information_schema.tables WHERE table_schema = 'public' AND table_name = 'SequelizeMeta'
/app/node_modules/@sequelize/core/lib/abstract-dialect/data-types.js:150
    throw new Error('The "key" instance property has been removed.');
          ^

Error: The "key" instance property has been removed.
    at get key (/app/node_modules/@sequelize/core/lib/abstract-dialect/data-types.js:150:11)
    at Sequelize.normalizeDataType (/app/node_modules/sequelize/lib/sequelize.js:561:27)
    at Sequelize.normalizeAttribute (/app/node_modules/sequelize/lib/sequelize.js:580:27)
    at /app/node_modules/sequelize/lib/dialects/abstract/query-interface.js:84:72
    at /app/node_modules/lodash/lodash.js:13469:38
    at /app/node_modules/lodash/lodash.js:4967:15
    at baseForOwn (/app/node_modules/lodash/lodash.js:3032:24)
    at Function.mapValues (/app/node_modules/lodash/lodash.js:13468:7)
    at PostgresQueryInterface.createTable (/app/node_modules/sequelize/lib/dialects/abstract/query-interface.js:84:20)
    at Object.up (file:///app/app/migrations/20250227.migration.js:5:26)

Node.js v22.14.0
/app # 

And here i stopped again, not knowing at all how to resolve this and where to start digging, but decided to place some of my research on the original issue here.

@WikiRik
Copy link
Member

WikiRik commented Feb 27, 2025

I see you are mixing the usage of sequelize v6 and v7. You need to either use sequelize (v6) or @sequelize/core and @sequelize/postgres (v7 alpha). That might not fix your error but it's the first thing I've noticed. If the error remains after removing sequelize I might be able to take a closer look at why the error occurs, we've changed quite a bit internally surrounding DataTypes in the v7 alphas and that might conflict with something

@MyNameIsNeXTSTEP
Copy link

MyNameIsNeXTSTEP commented Feb 27, 2025

@WikiRik I use sequelize (v6) for creating the context and the storage, and the @sequelize/core i used only for the DataTypes. I switched the use of types only to Sequelize like, so no @sequelize/core now, and i don't even have a @sequelize/postgres installed from the beginning:

import Sequelize from 'sequelize';

export default {
  async up({ context: queryInterface }) {
    await queryInterface.createTable('Users', {
      id: {
        type: Sequelize.INTEGER,
...

And again, even with await import(...) for resolving the migration script or using it even as .csj like:

const umzug = new Umzug({
  migrations: {
    glob: 'app/migrations/*.migration.js',
    resolve: async ({ name, path, context }) => {
      // const migration = await import(resolve(__dirname, path));
      const migration = require(path).default;
      console.log(migration) // Ouputs: { up: [AsyncFunction: up], down: [AsyncFunction: down] }
      return {
        name,
        up: () => migration.up({ context }),
        down: () => migration.down({ context }),
      };
    },
  },
  context: sequelize.getQueryInterface(),
  storage: new SequelizeStorage({ sequelize }),
  logger: console,
});

// export default umzug;
module.exports = umzug;

gives me the same error: MigrationError: Migration undefined (up) failed: Original error: m.up is not a function.\


UPD
What's more interensting, doing the same as in the screenshot above after switching to .cjs with require - the ValidationError occured, but the table was created from the up function in a migration.

P.S.
You may see it in my project, so i would not spam the code snippets here.
I referenced the issue in a commit, see below.
The repo (the init branch - would leave it as it is for now) - https://github.com/MyNameIsNeXTSTEP/simpleBackendApp/tree/init/app/migrations

MyNameIsNeXTSTEP added a commit to MyNameIsNeXTSTEP/simpleBackendApp that referenced this issue Feb 27, 2025
MyNameIsNeXTSTEP added a commit to MyNameIsNeXTSTEP/simpleBackendApp that referenced this issue Feb 27, 2025
@MyNameIsNeXTSTEP
Copy link

MyNameIsNeXTSTEP commented Mar 1, 2025

@WikiRik
I think figured out the problem, it's because to use it with ESM we need a new Umzug options: migrations -> resolve function to be async, but the current implementation only handles the synchronous one.

if we do in https://github.com/sequelize/umzug/blob/main/src/umzug.ts#L259 instead of just this:

await m.up(params)

Something like this:

const esmMigrationWithAsyncResolver = await m;
if (esmMigrationWithAsyncResolver) {
  esmMigrationWithAsyncResolver.up(params);
} else {
  await m.up(params);
}

And we change places with synchronous access to the m to async, for example this code at https://github.com/sequelize/umzug/blob/main/src/umzug.ts#L253:

const params: MigrationParams<Ctx> = {name: m.name, path: m.path, context}

to

const params: MigrationParams<Ctx> = {name: m.name || (await m).name, path: m.path, context}

to get rid of the error - "ValidationError: ... : SequelizeMeta.name cannot be null"

then it all works fine and set of migrations completes well.


So i assume, to fix this issue you need to work properly with the resolve migration option - https://github.com/sequelize/umzug/blob/main/src/types.ts#L89 being async and handle everywhere it's Promise-like nature.

@petemcfarlane
Copy link

To get around this I manually read and transformed my migrations files like so:

const migrations = fs.readdirSync(migrationsDir).map(name => {
  const { default: migration } = require(`./migrations/${name}`);
  return {
    up: async (params: MigrationParams<QueryInterface>) => await migration.up(params.context, sequelize),
    down: async (params: MigrationParams<QueryInterface>) => await migration.down(params.context, sequelize),
    name,
  };
});

const umzug = new Umzug({
  migrations,
  context: sequelize.getQueryInterface(),
  storage: new SequelizeStorage({ sequelize }),
  logger: console,
});

Not ideal, but it worked!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants