Skip to content

Return a nonzero exit status for detected FASTQ parsing errors - #718

Open
thierrygosselin wants to merge 2 commits into
OpenGene:masterfrom
thierrygosselin:fix-fastq-parse-error-exit
Open

Return a nonzero exit status for detected FASTQ parsing errors#718
thierrygosselin wants to merge 2 commits into
OpenGene:masterfrom
thierrygosselin:fix-fastq-parse-error-exit

Conversation

@thierrygosselin

Copy link
Copy Markdown

Summary

When the FASTQ parser detects an invalid separator or unequal sequence and quality lengths, it currently returns NULL. Callers interpret this as normal EOF, allowing incomplete processing to finish with exit status 0.

This PR replaces those two error returns with error_exit().

Changes

  • Fail explicitly on invalid or missing FASTQ separators.
  • Fail explicitly on sequence/quality length mismatches.
  • Preserve existing record diagnostics and include the input filename.
  • Leave normal EOF handling unchanged.

This is separate from #533, which addresses filesystem read errors. It does not depend on #716 or #717.

Regression tests

Added tests for plain and gzipped FASTQ containing:

  • Invalid or missing separators.
  • Short, long, or missing quality strings.
  • Errors at the first record or after 40,000 valid reads.
  • Valid input, including files without a final newline.

Run with:

python3 scripts/test_fastq_parse_errors.py ./fastp

On macOS ARM64, all 24 cases pass with the patch. Unmodified Conda fastp 1.3.6 incorrectly returns success for all 20 malformed-input cases; the four valid-input cases pass.

Scope

This changes the exit behaviour of existing parser checks, not the full FASTQ validation policy. Filesystem I/O failures and other malformed-record patterns are outside this patch. Linux execution has not been tested.

Use error_exit instead of returning NULL when a FASTQ record has an
invalid separator or unequal sequence and quality lengths.

Returning NULL made these detected errors indistinguishable from normal
EOF, allowing incomplete processing to finish with exit status 0.

Preserve existing record diagnostics and include the input filename
in the fatal error message. This is separate from filesystem read-error
handling proposed in PR OpenGene#533.
Test invalid or missing separators and short, long, or missing quality
strings in plain and gzipped FASTQ files.

Place malformed records both at the beginning and after 40,000 valid
reads. Verify that valid input, including files without a final newline,
continues to work.

All 24 cases pass with the fix. Unmodified Conda fastp 1.3.6 incorrectly
returns success for all 20 malformed-input cases.
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

Successfully merging this pull request may close these issues.

1 participant