j-easy / easy-batch

The simple, stupid batch framework for Java
https://github.com/j-easy/easy-batch/wiki
MIT License
611 stars 199 forks source link

Errors in reader/writer open/close methods are not handled #393

Closed cn-src closed 3 years ago

cn-src commented 3 years ago

When java.lang.Error is thrown in the RecordReader open() method, the error information will be lost, and the toString() method of JobReport will generate a NullPointerException because getStartTime() is null.

fmbenhassine commented 3 years ago

Thank you for opening this issue. However, I don't understand how a NPE could happen as the start method (where the startTime is set) is called before the openReader method.

I was not able to reproduce the issue, here is a quick example:

import org.jeasy.batch.core.job.Job;
import org.jeasy.batch.core.job.JobBuilder;
import org.jeasy.batch.core.job.JobReport;
import org.jeasy.batch.core.reader.RecordReader;
import org.jeasy.batch.core.record.Record;
import org.jeasy.batch.core.writer.StandardOutputRecordWriter;

public class Launcher {

    public static void main(String[] args) {
        Job job = new JobBuilder<String, String>()
                .reader(new RecordReader<>() {
                    @Override
                    public Record<String> readRecord() {
                        return null;
                    }

                    @Override
                    public void open() {
                        throw new Error("boom");
                    }
                })
                .writer(new StandardOutputRecordWriter<>())
                .build();
        JobReport jobReport = job.call();
        System.out.println(jobReport);
    }

}

This example does not fail with a NPE. However, it crashes with:

[main] INFO org.jeasy.batch.core.job.BatchJob - Job 'job' starting
Exception in thread "main" java.lang.Error: boom
    at org.jeasy.batch.tutorials.basic.helloworld.Launcher$1.open(Launcher.java:56)
    at org.jeasy.batch.core.job.BatchJob.openReader(BatchJob.java:151)
    at org.jeasy.batch.core.job.BatchJob.call(BatchJob.java:107)
    at org.jeasy.batch.tutorials.basic.helloworld.Launcher.main(Launcher.java:61)

Is this what you mean by the error information will be lost? Anyway, Easy Batch should not crash like this (currently it only catches exceptions but not errors when opening/closing the reader/writer), it should catch errors and stops gracefully, so I consider this as a bug.

fmbenhassine commented 3 years ago

I pushed a fix (see 2c0b7ca) and the previous example now fails gracefully like follows:

[main] INFO org.jeasy.batch.core.job.BatchJob - Job 'job' starting
[main] ERROR org.jeasy.batch.core.job.BatchJob - Unable to open record reader
java.lang.Error: boom
    at org.jeasy.batch.tutorials.basic.helloworld.Launcher$1.open(Launcher.java:46)
    at org.jeasy.batch.core.job.BatchJob.openReader(BatchJob.java:151)
    at org.jeasy.batch.core.job.BatchJob.call(BatchJob.java:107)
    at org.jeasy.batch.tutorials.basic.helloworld.Launcher.main(Launcher.java:51)
[main] INFO org.jeasy.batch.core.job.BatchJob - Job 'job' finished with status FAILED in 4ms
Job Report:
===========
Name: job
Status: FAILED
Parameters:
    Batch size = 100
    Error threshold = N/A
    Jmx monitoring = false
    Batch scanning = false
Metrics:
    Start time = 2020-11-17T23:33:53.76051
    End time = 2020-11-17T23:33:53.764532
    Duration = 4ms
    Read count = 0
    Write count = 0
    Filter count = 0
    Error count = 0

Can you give it a try in v7.0.1-SNAPSHOT and share your feedback?

cn-src commented 3 years ago

Yes, I can get normal error messages now. thank you!

fmbenhassine commented 3 years ago

@cn-src FYI, the fix for this issue has been released in v7.0.1.