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

Provide a special class for pipeline-defined error messages #4937

Open
jfy133 opened this issue Apr 20, 2024 · 4 comments
Open

Provide a special class for pipeline-defined error messages #4937

jfy133 opened this issue Apr 20, 2024 · 4 comments

Comments

@jfy133
Copy link
Contributor

jfy133 commented Apr 20, 2024

New feature

It would be nice to have a special error class or function for pipeline-level errors or warnings, that help distinguish pipeline-defined (conditional) errors versus Nextflow level errors

Usage scenario

Pipelines will often have their own checks or validations, for example mutually exclusive parameters, that they will want to report to a user. Currently it is expected that a pipeline uses error("error message") to report this.

Often these are 'simple' errors, compared to Nextflow errors that have a more technical reason for the problem.

It might nice to be able to distinguish such pipeline-level errors from Nextflow errors. One benefit would be that it makes it much faster to identify where the error is coming from for developers, so they can quickly guide a user to a solution (i.e., whether it's a simple fix, or something the developer needs to investigate deeper.

Currently I do this manually by adding a prefix to warnings or logs, but it could be nice to have this happen automatically. Furthermore the way to report an error vs warning is slightly different (to my understanding anyway).

e.g. to report an error I do the following with the pipeline name as a prefix:

if (params.shortread_qc_includeunmerged && !params.shortread_qc_mergepairs) error("ERROR: [nf-core/taxprofiler] cannot include unmerged reads when merging is not turned on. Please specify --shortread_qc_mergepairs")

https://github.com/nf-core/taxprofiler/blob/45ce748534872c5105a5d3a94b83f47cd44f8826/workflows/taxprofiler.nf#L36

vs a warning

if (params.diamond_save_reads              ) log.warn "[nf-core/taxprofiler] DIAMOND only allows output of a single format. As --diamond_save_reads supplied, only aligned reads in SAM format will be produced, no taxonomic profiles will be available."

https://github.com/nf-core/taxprofiler/blob/45ce748534872c5105a5d3a94b83f47cd44f8826/workflows/taxprofiler.nf#L48

Where I have to manually specify ERROR: in when using error(), but the log.warn() autmatically puts WARN at the beginning. I guess there is a log.error but my understanding this doesn't cause the pipeline run to exit 'nicely'.

Suggest implementation

Could be nice to have something a native pipelineError("") - thing the prefixes the workflow name at the beginning etc.

It could also have a distinct colour from a Nextflow level error.

More info

I initially brought this up for discussion with nf-core (in particular @ewels and @maxulysse) here: https://nfcore.slack.com/archives/C04QR0T3G3H/p1713533132261319 based on the suggestion from @Midnighter here: nf-core/taxprofiler#60 (comment).

All three may be able to provide better (technical) suggestions and/or benefits.

@ewels
Copy link
Member

ewels commented Apr 21, 2024

Optimum would be if Nextflow can determine where the error calls come from and figure out if it's coming from pipeline source code. Could then prefix with manifest.name if set.

Then no change to pipeline source code would be required.

@Midnighter
Copy link

These are two slightly different beasts. For the logger, it could be as simple as modifying the layout of messages. Although, we would then need two different loggers, one (or more) for core nextflow and another exposed to the pipeline that can be customized without affecting the others.

I don't know for sure, but I'm guessing that errors do not use logging facilities and thus need to be customized separately. Or indeed define a simple function/class method that performs some message layouting and then exits with an error code.

@jfy133
Copy link
Contributor Author

jfy133 commented Apr 21, 2024

Yes, sorry to be clear: I'm aware that the logging and error message are generated in two separate ways.

I meant more that in this case, given I'm requesting that pipeline developers can define their own errors or warnings, these should follow a common structure for familiarity for the pipeline user.

@mahesh-panchal
Copy link
Contributor

Adding my comment here:
nf-core/fetchngs#309 (comment)

One of my issues was that something ends up in the channel too if ifEmpty is used. Although this might well be solved with DSL3.

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

No branches or pull requests

4 participants