adamkewley / jobson

A platform for transforming command-line applications into a job service.
Apache License 2.0
256 stars 20 forks source link

Absolute job output throws exception if missing #43

Closed adamkewley closed 6 years ago

adamkewley commented 6 years ago

Found by a user. The server throws an exception internally if the job's expected output is an absolute path (e.g. /some/path):

Exception in thread "Thread-26" java.lang.IllegalArgumentException: 'other' is different type of Path
at sun.nio.fs.UnixPath.relativize(UnixPath.java:416)
at sun.nio.fs.UnixPath.relativize(UnixPath.java:43)
at com.github.jobson.jobs.LocalJobExecutor.tryGetJobOutput(LocalJobExecutor.java:241)
at com.github.jobson.jobs.LocalJobExecutor.lambda$tryResolveJobOutputs$6(LocalJobExecutor.java:220)
at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1382)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
at com.github.jobson.jobs.LocalJobExecutor.tryResolveJobOutputs(LocalJobExecutor.java:222)
at com.github.jobson.jobs.LocalJobExecutor.onProcessExit(LocalJobExecutor.java:194)
at com.github.jobson.jobs.LocalJobExecutor.lambda$execute$5(LocalJobExecutor.java:174)
at com.github.jobson.Helpers.lambda$attachTo$10(Helpers.java:339)
at java.lang.Thread.run(Thread.java:748)

This is caused when using an absolute path, but that path is missing. It happens because Java's Path.relativize method throws an exception if you mix relative an absolute paths together:

private JobOutputResult tryGetJobOutput(Path workingDir, PersistedJob job, JobOutputId outputId, JobExpectedOutput expectedOutput) {
        final Path expectedOutputFile = workingDir.resolve(resolveArg(job, workingDir, expectedOutput.getPath()));

        if (expectedOutputFile.toFile().exists()) {
            final String mimeType = establishMimeType(expectedOutput, expectedOutputFile);
            final BinaryData data = streamBinaryData(expectedOutputFile, mimeType);
            return new JobOutput(
                    outputId,
                    data,
                    expectedOutput.getName(),
                    expectedOutput.getDescription(),
                    expectedOutput.getMetadata());
        } else {
            return new MissingOutput(
                    outputId,
                    expectedOutput.isRequired(),
                    expectedOutputFile.relativize(workingDir).toString());
        }
    }

The relativize call in this instance is a bit frivolous, because the expectedOutputFile is already resolved against the working directory. Therefore, it should be possible to just remove the call and prevent that exception.

adamkewley commented 6 years ago

Investigating this more, it reveals that job output wrangling will always fail if the output is absolute but the working directory is relative.

By default, Jobson's unit/system tests are using absolute paths, so this wasn't smoked out. Once I updated the test suites to use relative paths, it revealed that this one like (.relativize) would take down many of the tests (absolute path) jobs.

I updated the tests and patched the code accordingly - eveything's passing again. This will be submitted onto master and rolled out as 0.0.21.