search

yogthos / migratus

MIGRATE ALL THE THINGS!
642 stars 93 forks source link

A dot in the migration file name causes obscure errors #173

Open abeluck opened 5 years ago

abeluck commented 5 years ago

When a migration file has a dot in the filename, migratus fails with some very unhelpful exceptions.

I ran into this while working on a new luminus project and spent a good 30 minutes debugging before I figured out the issue. Looks like this was the root cause in #152 as well, but the issue was never fixed.

If it's not desired to support dots in filenames, then perhaps the create-migration function could error or munge the dots and the migrate function should throw a helpful error message.

The current error is:

java.lang.ClassCastException: class clojure.lang.PersistentArrayMap cannot be cast to class java.lang.CharSequence (clojure.lang.PersistentArrayMap is in unnamed module of loader 'app'; java.lang.CharSequence is in module java.base of loader 'bootstrap')
    at migratus.migration.sql$split_commands.invokeStatic(sql.clj:26)
    at migratus.migration.sql$split_commands.invoke(sql.clj:24)
    at migratus.migration.sql$run_sql.invokeStatic(sql.clj:76)
    at migratus.migration.sql$run_sql.invoke(sql.clj:74)
    at migratus.migration.sql.SqlMigration.down(sql.clj:101)
    at migratus.database$migrate_up_STAR_$fn__31204.invoke(database.clj:70)
    at migratus.database$migrate_up_STAR_.invokeStatic(database.clj:69)
    at migratus.database$migrate_up_STAR_.invoke(database.clj:55)
    at migratus.database.Database$fn__31270.invoke(database.clj:247)
yogthos commented 5 years ago

I've updated the check for parsing the migration file names with a better heuristic to see whether the file was parsed correctly. Just pushed out 1.2.6 with the update that should help debugging the issue. Unfortunately, I just realized that it's a bit trickier than I originally thought. The file format is actually dependent on the specific migration implementation. For example, currently there's EDN and SQL file support. The EDN version can't have . characters except to separate the extension, while the SQL version has to end with .up|down.sql.

So, the proper fix here would be to select the file pattern based on the migration implementation being used. I'll try take a look at this when I get a chance, but a PR would be welcome as well.