From bd948e06c491b41b13660918a7171ef6332028e2 Mon Sep 17 00:00:00 2001 From: Brian Pratt Date: Mon, 9 Oct 2023 20:18:44 -0700 Subject: [PATCH 1/2] Deal with an unhandled exception in small molecule transition list reader by making exception handling more consistent throughout --- .../SmallMoleculeTransitionListReader.cs | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/pwiz_tools/Skyline/Model/SmallMoleculeTransitionListReader.cs b/pwiz_tools/Skyline/Model/SmallMoleculeTransitionListReader.cs index 6a39469779..77b86cc056 100644 --- a/pwiz_tools/Skyline/Model/SmallMoleculeTransitionListReader.cs +++ b/pwiz_tools/Skyline/Model/SmallMoleculeTransitionListReader.cs @@ -296,13 +296,11 @@ private bool ErrorFindingTransitionGroupForPrecursor(ref SrmDocument document, P _firstAddedPathPepGroup = _firstAddedPathPepGroup ?? pathGroup; } } - catch (InvalidDataException x) - { - errmsg = x.Message; - } - catch (InvalidOperationException x) // Adduct handling code can throw these + + catch (Exception exception) { - errmsg = x.Message; + ThrowIfNotParserError(exception); + errmsg = exception.Message; } if (errmsg != null) @@ -1011,6 +1009,18 @@ private void GetMoleculeDetails() } } + private void ThrowIfNotParserError(Exception exception) + { + if (exception is InvalidOperationException + || exception is InvalidDataException + || exception is ArgumentException) + { + return; + } + + throw exception; + } + // We need some combination of: // Formula and mz // Formula and charge @@ -1511,13 +1521,10 @@ private ParsedIonInfo ReadPrecursorOrProductColumns(SrmDocument document, } } } - catch (InvalidDataException x) - { - massErrMsg = x.Message; - } - catch (InvalidOperationException x) // Adduct handling code can throw these + catch (Exception exception) { - massErrMsg = x.Message; + ThrowIfNotParserError(exception); + massErrMsg = exception.Message; } if (massErrMsg != null) { @@ -1704,8 +1711,9 @@ private bool ProcessAdduct(Row row, int indexAdduct, int indexFormula, bool getP adduct = Adduct.FromStringAssumeChargeOnly(adductText); IonInfo.ApplyAdductToFormula(formula ?? string.Empty, adduct); // Just to see if it throws } - catch (Exception x) when ((x is InvalidOperationException) || (x is ArgumentException)) + catch (Exception x) { + ThrowIfNotParserError(x); ShowTransitionError(new PasteError { Column = indexAdduct, @@ -1831,8 +1839,9 @@ private PeptideDocNode GetMoleculePeptide(SrmDocument document, Row row, Peptide molecule = new CustomMolecule(parsedIonInfo.MonoMass, parsedIonInfo.AverageMass, shortName, parsedIonInfo.MoleculeID.AccessionNumbers); } } - catch (ArgumentException e) + catch (Exception e) { + ThrowIfNotParserError(e); ShowTransitionError(new PasteError { Column = INDEX_MOLECULE_FORMULA, @@ -1849,8 +1858,9 @@ private PeptideDocNode GetMoleculePeptide(SrmDocument document, Row row, Peptide return null; return new PeptideDocNode(pep, document.Settings, null, null, parsedIonInfo.ExplicitRetentionTime, new[] { tranGroup }, false); } - catch (InvalidOperationException e) + catch (Exception e) { + ThrowIfNotParserError(e); ShowTransitionError(new PasteError { Column = INDEX_MOLECULE_FORMULA, @@ -1908,12 +1918,9 @@ private TransitionGroupDocNode GetMoleculeTransitionGroup(SrmDocument document, return new TransitionGroupDocNode(group, document.Annotations, document.Settings, null, null, moleculeInfo.ExplicitTransitionGroupValues, null, new[] { tran }, false); } - catch (InvalidDataException x) - { - errmsg = x.Message; - } - catch (InvalidOperationException x) // Adduct handling code can throw these + catch (Exception x) { + ThrowIfNotParserError(x); errmsg = x.Message; } ShowTransitionError(new PasteError From 8af7a163756493b8fa7fe9e35c0807f52f15ac22 Mon Sep 17 00:00:00 2001 From: Brian Pratt Date: Tue, 10 Oct 2023 08:10:39 -0700 Subject: [PATCH 2/2] tidier multi-catch implementation --- .../SmallMoleculeTransitionListReader.cs | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/pwiz_tools/Skyline/Model/SmallMoleculeTransitionListReader.cs b/pwiz_tools/Skyline/Model/SmallMoleculeTransitionListReader.cs index 77b86cc056..dbbd19f317 100644 --- a/pwiz_tools/Skyline/Model/SmallMoleculeTransitionListReader.cs +++ b/pwiz_tools/Skyline/Model/SmallMoleculeTransitionListReader.cs @@ -254,6 +254,15 @@ private bool ErrorAddingToExistingMoleculeGroup(ref SrmDocument document, Parsed return false; } + // Check for the various kinds of exceptions that may be thrown in the course of + // parsing sometimes-wonky user data + private bool IsParserException(Exception exception) + { + return exception is InvalidOperationException + || exception is InvalidDataException + || exception is ArgumentException; + } + // Returns true on error private bool ErrorFindingTransitionGroupForPrecursor(ref SrmDocument document, ParsedIonInfo precursor, Row row, PeptideGroupDocNode pepGroup, Adduct adduct, double precursorMonoMz, double precursorAverageMz, @@ -296,10 +305,8 @@ private bool ErrorFindingTransitionGroupForPrecursor(ref SrmDocument document, P _firstAddedPathPepGroup = _firstAddedPathPepGroup ?? pathGroup; } } - - catch (Exception exception) + catch (Exception exception) when (IsParserException(exception)) { - ThrowIfNotParserError(exception); errmsg = exception.Message; } @@ -1009,18 +1016,6 @@ private void GetMoleculeDetails() } } - private void ThrowIfNotParserError(Exception exception) - { - if (exception is InvalidOperationException - || exception is InvalidDataException - || exception is ArgumentException) - { - return; - } - - throw exception; - } - // We need some combination of: // Formula and mz // Formula and charge @@ -1367,7 +1362,7 @@ private ParsedIonInfo ReadPrecursorOrProductColumns(SrmDocument document, adduct = DetermineAdductFromFormulaChargeAndMz(formula, charge.Value, mz); } } - catch (Exception e) + catch (Exception e) when (IsParserException(e)) { ShowTransitionError(new PasteError { @@ -1521,9 +1516,8 @@ private ParsedIonInfo ReadPrecursorOrProductColumns(SrmDocument document, } } } - catch (Exception exception) + catch (Exception exception) when (IsParserException(exception)) { - ThrowIfNotParserError(exception); massErrMsg = exception.Message; } if (massErrMsg != null) @@ -1711,9 +1705,8 @@ private bool ProcessAdduct(Row row, int indexAdduct, int indexFormula, bool getP adduct = Adduct.FromStringAssumeChargeOnly(adductText); IonInfo.ApplyAdductToFormula(formula ?? string.Empty, adduct); // Just to see if it throws } - catch (Exception x) + catch (Exception x) when (IsParserException(x)) { - ThrowIfNotParserError(x); ShowTransitionError(new PasteError { Column = indexAdduct, @@ -1839,9 +1832,8 @@ private PeptideDocNode GetMoleculePeptide(SrmDocument document, Row row, Peptide molecule = new CustomMolecule(parsedIonInfo.MonoMass, parsedIonInfo.AverageMass, shortName, parsedIonInfo.MoleculeID.AccessionNumbers); } } - catch (Exception e) + catch (Exception e) when (IsParserException(e)) { - ThrowIfNotParserError(e); ShowTransitionError(new PasteError { Column = INDEX_MOLECULE_FORMULA, @@ -1858,9 +1850,8 @@ private PeptideDocNode GetMoleculePeptide(SrmDocument document, Row row, Peptide return null; return new PeptideDocNode(pep, document.Settings, null, null, parsedIonInfo.ExplicitRetentionTime, new[] { tranGroup }, false); } - catch (Exception e) + catch (Exception e) when (IsParserException(e)) { - ThrowIfNotParserError(e); ShowTransitionError(new PasteError { Column = INDEX_MOLECULE_FORMULA, @@ -1918,9 +1909,8 @@ private TransitionGroupDocNode GetMoleculeTransitionGroup(SrmDocument document, return new TransitionGroupDocNode(group, document.Annotations, document.Settings, null, null, moleculeInfo.ExplicitTransitionGroupValues, null, new[] { tran }, false); } - catch (Exception x) + catch (Exception x) when (IsParserException(x)) { - ThrowIfNotParserError(x); errmsg = x.Message; } ShowTransitionError(new PasteError