fix(scanner): report a not-found barcode as not found - #712
Open
EugeneSusla wants to merge 4 commits into
Open
Conversation
The failure branch compared the caught exception object against the ProductNotFoundException *type*, which is never equal, so the productNotFound state was unreachable and every OFF 404 surfaced as "Error while fetching product data" with a Retry button that could not help. Use an `is` check so the existing errorProductNotFound string and its UI wiring are actually reached. Adds a ScannerBloc test covering both failure classifications.
Both cases asserted only on the emitted failure type, which the bloc also produces for an unexpected throw from anywhere inside its try — a misused fake, or a future reordering that queries config before the search. The helper now asserts the barcode reached the search use case, and the seeded error is a required non-null constructor argument so a null-deref cannot stand in for it. Also asserts the settled state is a failure instead of filtering the stream for one, with a timeout, so a bloc that loads a product or emits nothing fails with a readable diff rather than hanging.
The config fake threw, and the bloc queries config inside the same try as the search, so a swallowed search exception still surfaced as a generic failure — the state assertion could never see ScannerLoadedState and the comment claiming otherwise was wrong. Return a real ConfigEntity, matching the fake in scanner_screen_pick_mode_test.dart, so dropping the exception now fails with a readable diff. Also keeps the bloc nullable so a future non-scanning test cannot trip a LateInitializationError in teardown, and names the settle timeout with the reason it cannot race a loaded machine.
30s matched package:test's own default per-test timeout, so a hung stream would likely have failed as the harness's generic timeout rather than the diagnostic this guard exists to raise. Back to 5s, with the constraint written down. Also closes every bloc a test builds rather than just the last one, and notes that the config fake's values are placeholders — the bloc only reads usesImperialFoodUnits, and only on a path these tests never take.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Scanning a barcode that Open Food Facts does not have shows "Error while fetching product data" with a Retry button, rather than the "Product not found" message the app already ships.
ScannerBlocclassifies the failure with:That compares the caught exception object against the type, which is never equal. The
productNotFoundbranch is therefore unreachable, and every OFF 404 falls through to the generic error state.errorProductNotFoundand its wiring inscanner_screen.dartwere already in place and simply never reached.The Retry button is especially misleading here:
OFFDataSourcedeliberately does not retry a not-found (shouldRetry: (e) => e is! ProductNotFoundException), so retrying a 404 can never succeed.Reproduces with any barcode absent from OFF — for example an in-store GS1-128 label, whose GTIN is restricted-circulation and so never registered globally.
Change
exception == ProductNotFoundException→exception is ProductNotFoundException.ScannerBloctests covering both failure classifications.The test asserts the barcode actually reached the search use case, and that the settled state is a failure rather than filtering the stream for one, so a swallowed exception fails with a readable diff instead of passing on the bloc's catch-all.
Verification
flutter analyzeclean; full suite green (964 tests).