tools icon indicating copy to clipboard operation
tools copied to clipboard

Don't use `System.exit(1)`

Open ewels opened this issue 3 years ago • 1 comments

We've been giving @pditommaso nightmares with our use of System.exit:

https://github.com/nf-core/tools/blob/bc56cdd35077e408a0a7c90ec07fcdff7c2d1f3b/nf_core/pipeline-template/lib/WorkflowPipeline.groovy#L16-L17

$ nextflow run .
N E X T F L O W  ~  version 22.08.2-edge
Launching `./main.nf` [crazy_swirles] DSL2 - revision: 6b60f60cc2
Genome fasta file not specified with e.g. '--fasta genome.fa' or via a detectable config file.

Instead, we should be using exceptions:

throw new Exception("Genome fasta file not specified with e.g. '--fasta genome.fa' or via a detectable config file.")
$ nextflow run .
N E X T F L O W  ~  version 22.08.2-edge
Launching `./main.nf` [cranky_roentgen] DSL2 - revision: 62f715c40c
Genome fasta file not specified with e.g. '--fasta genome.fa' or via a detectable config file.

 -- Check script 'main.nf' at line: 1 or see '.nextflow.log' file for more details

TODO

  • [ ] Remove from the template
  • [ ] Add lint tests that search all files for System.exit

ewels avatar Sep 26 '22 10:09 ewels

An easier method is to use error which is a wrapper around calling an exception.

error("Genome fasta file not specified with e.g. '--fasta genome.fa' or via a detectable config file.")

ewels avatar Sep 26 '22 14:09 ewels

Cannot push my code right now due to some GitHub issue but I had a go at writing some lint code:

╭─ [!] 6 Pipeline Test Warnings 
│ system_exit: System.exit in WorkflowNanoporeqc.groovy: System.exit(1)                                                                                                                            
│ system_exit: System.exit in WorkflowNanoporeqc.groovy: System.exit(1)                                                                                                                            
│ system_exit: System.exit in NfcoreSchema.groovy: System.exit(1)                                                                                                                                  
│ system_exit: System.exit in WorkflowMain.groovy: System.exit(0)                                                                                                                                  
│ system_exit: System.exit in WorkflowMain.groovy: System.exit(0)                                                                                                                                  
│ system_exit: System.exit in WorkflowMain.groovy: System.exit(1)                                                                                                                                  

So I think this should inform the template changes to make too.

awgymer avatar Mar 27 '23 12:03 awgymer

See commit for how we should replace System.exit(1) calls in the pipeline template with Nextflow.error(message). Will need to update the Schema validation script too.

At the moment Nextflow still generates the -- Check script message which we don't really want because it could cause confusion with users because the failure is expected:

* Software dependencies
  https://github.com/nf-core/rnaseq/blob/master/CITATIONS.md
------------------------------------------------------
Genome fasta file not specified with e.g. '--fasta genome.fa' or via a detectable config file.

 -- Check script './workflows/rnaseq.nf' at line: 17 or see '.nextflow.log' file for more details

I asked Paolo to fix this just now and it's done so will be available in the next NF release: https://github.com/nextflow-io/nextflow/commit/38c8bd335d286ddc3907254b9d6817581f2a67d0

drpatelh avatar Mar 27 '23 15:03 drpatelh

Closed in #2211

awgymer avatar Mar 28 '23 14:03 awgymer