Skip to content

Conversation

hl662
Copy link
Contributor

@hl662 hl662 commented Sep 30, 2025

This pull request improves error handling in the @itwin/core-quantity package's quantity parser. The parser now returns an explicit error result when a unit label is supplied but cannot be matched to any known unit, rather than silently failing or accepting invalid labels. This is a breaking change that ensures more accurate feedback for consumers of the parsing API.

Error handling improvements:

  • The Parser class now throws a QuantityError with QuantityStatus.UnitLabelSuppliedButNotMatched when unique unit labels are present but not matched to any known unit.
  • The main parsing logic catches this error and returns a result with ParseError.UnitLabelSuppliedButNotMatched, ensuring callers receive a clear error when parsing fails due to unknown unit labels.

Testing and documentation:

  • Added a new test case to Parsing.test.ts to verify that parsing with invalid unit labels returns the expected error.
  • Updated the change history documentation to highlight this, warning that parsing operations with invalid unit labels will now return errors instead of succeeding silently.

@hl662 hl662 self-assigned this Sep 30, 2025
@hl662
Copy link
Contributor Author

hl662 commented Sep 30, 2025

I will highlight that none of our previous tests failed, there are no regressions so far, including this relevant test case where when a unitless format is passed in, the parser will use the first unit label that matches any known units for parsing

const testData = [
  { value: "12,345.345 - 1", magnitude: 12345.345 - 1}, // unitless
  { value: "1m + 1FT + 1", magnitude: 1 + 0.3048 + 1}, // default to meters
  { value: "1FT + 1m + 1", magnitude: 0.3048 + 1 + 0.3048}, // default to feet
  { value: "1 + 1 FT + 1m", magnitude: 0.3048 + 0.3048 + 1 }, // default to feet
];

@hl662 hl662 requested a review from Copilot September 30, 2025 15:00
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves error handling in the quantity parser by returning explicit error results when unit labels cannot be matched to known units, replacing the previous silent failure behavior.

  • Enhanced the Parser.getDefaultUnitConversion method to throw a QuantityError when unit labels are present but unmatched
  • Updated the main parsing logic to catch these errors and return ParseError.UnitLabelSuppliedButNotMatched results
  • Added comprehensive test coverage for invalid unit label scenarios

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
core/quantity/src/Parser.ts Added error throwing for unmatched unit labels and error handling in parsing logic
core/quantity/src/test/Parsing.test.ts Added test cases to verify proper error handling for invalid unit labels
docs/changehistory/NextVersion.md Documented the breaking change for enhanced error handling
common/changes/@itwin/core-quantity/nam-parser-label-fix_2025-09-30-14-41.json Added change log entry for the parser enhancement

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@hl662 hl662 marked this pull request as ready for review September 30, 2025 15:38
@hl662 hl662 requested review from a team, ColinKerr and rschili as code owners September 30, 2025 15:38
@hl662 hl662 enabled auto-merge (squash) October 14, 2025 18:55
@hl662 hl662 merged commit 3a2d775 into master Oct 14, 2025
15 checks passed
@hl662 hl662 deleted the nam/parser-label-fix branch October 14, 2025 19:37
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.

5 participants