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

Need a way to find out from FlywayException which SQL command failed #548

Closed
bpg opened this issue Jul 8, 2013 · 4 comments
Closed

Need a way to find out from FlywayException which SQL command failed #548

bpg opened this issue Jul 8, 2013 · 4 comments

Comments

@bpg
Copy link

@bpg bpg commented Jul 8, 2013

This feature request partially duplicates #234 bug.

I would like to have ability to obtain some details regarding failed SQL statements from the generated FlywayException.
For example, the code:

try{
    Flyway flyway = new Flyway();
    flyway.setDataSource(ds);
    flyway.migrate();
} catch (FlywayException ex) {
    // process ex;
}

produces exception like:

Caused by: com.googlecode.flyway.core.api.FlywayException: Migration of schema "DEV" to version 3.0.0.001 failed! Please restore backups and roll back database and code!
    at com.googlecode.flyway.core.command.DbMigrate.migrate(DbMigrate.java:199)
    at com.googlecode.flyway.core.Flyway$1.execute(Flyway.java:866)
    at com.googlecode.flyway.core.Flyway$1.execute(Flyway.java:819)
...

But I want to tell end user (in my case it is a system administrator) what exactly happened during the migration and give her ability to fix the situation and repeat the attempt.
The current implementation gives only one way to know about the errors -- the log file (tested with Oracle DB):

1775 [main] ERROR com.googlecode.flyway.core.command.DbMigrate  - com.googlecode.flyway.core.api.FlywayException: Error executing statement at line 17: CREATE GLOBAL TEMPORARY TABLE "TEMP_XXX_INSERTED" (
  "ID" INTEGER NOT NULL,
  CONSTRAINT "TMP_XXX_INSRTD" PRIMARY KEY ("ID")
) ON COMMIT DELETE ROWS>

1775 [main] ERROR com.googlecode.flyway.core.command.DbMigrate  - Caused by java.sql.SQLException: ORA-00922: missing or invalid option

I would like to see an extension of FlywayException interface (or subclass e.g. FlywayMigrationException) that can give me a list of errors occurred during migration (or only the first error -- because Flyway does not apply subsequent migrations), so it can be processed by business logic.

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Jul 9, 2013

Thanks for the report. I agree with you, this would make total sense from an API perspective. I'll look into it.

@pentike
Copy link

@pentike pentike commented Jul 11, 2013

Hi Axel,

it seems to me the problem is with the e.toString() call instead of the use of e.getStackTrace() in applyMigration():

private Pair<Boolean, MigrationVersion> applyMigration(final ResolvedMigration migration, boolean isOutOfOrder) throws FlywayException {
...
} catch (Exception e) {
    LOG.error(e.toString());

    @SuppressWarnings({"ThrowableResultOfMethodCallIgnored"})
    Throwable rootCause = ExceptionUtils.getRootCause(e);
    if (rootCause != null) {
        LOG.error("Caused by " + rootCause.toString());
    }
}

The stack trace is swallowed and that makes important information such as the line number of the exception disappear.

Best regards,
Gábor

@bpg
Copy link
Author

@bpg bpg commented Jul 12, 2013

Hi there,

Logging of the exception is not an issue, rather I would like to get the actual exception raised up to the top level FlywayException (wrapped by or included into it).
As far as I understand it is not a trivial task, but I believe this feature will be greatly appreciated by developers of enterprise-grade software, where the full control over every aspect of functionality is usually required.

Thanks,
Pavel.

@pentike
Copy link

@pentike pentike commented Jul 12, 2013

Hi!

I have created a quick solution for the issue.

Add this method to ExceptionUtils:

    /**
     * Returns the StackTrace of this throwable as a string. Each lines are separated by a newline (\n) character.
     * 
     * @param throwable
     * @return String representation of the StackTrace
     */
    public static String getStackTrace(Throwable throwable) {
        StringBuilder sb = new StringBuilder(2000);
        StackTraceElement[] stackTrace = throwable.getStackTrace();
        boolean first = true;
        for (StackTraceElement ste : stackTrace) {
            if(!first) sb.append("\n\t at ");
            sb.append(ste.toString());
            first=false;
        }
        return sb.toString();
    }

and change DBMigrate#applyMigration (lines 264-266) to

            if (rootCause != null) {
                LOG.error("Caused by " + ExceptionUtils.getStackTrace(rootCause));
            }

Best regards,
Gábor

axelfontaine added a commit that referenced this issue Jul 14, 2013
Implementation of #548 feature request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.