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

Don't use System.exit(1) #1840

Closed
2 tasks
ewels opened this issue Sep 26, 2022 · 4 comments · Fixed by #2211
Closed
2 tasks

Don't use System.exit(1) #1840

ewels opened this issue Sep 26, 2022 · 4 comments · Fixed by #2211
Assignees
Labels

Comments

@ewels
Copy link
Member

ewels commented Sep 26, 2022

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

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

$ 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 ewels added template nf-core pipeline/component template linting labels Sep 26, 2022
@ewels
Copy link
Member Author

ewels commented Sep 26, 2022

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.")

@awgymer
Copy link
Contributor

awgymer commented Mar 27, 2023

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 awgymer self-assigned this Mar 27, 2023
@awgymer awgymer moved this from Todo to In Progress in nf-core Hackathon March 2023 Mar 27, 2023
@awgymer awgymer linked a pull request Mar 27, 2023 that will close this issue
4 tasks
@drpatelh
Copy link
Member

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:
nextflow-io/nextflow@38c8bd3

@awgymer
Copy link
Contributor

awgymer commented Mar 28, 2023

Closed in #2211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants