Skip to content

catch more specific errors#189

Open
kiftio wants to merge 1 commit into
mainfrom
05-22-catch_more_specific_errors
Open

catch more specific errors#189
kiftio wants to merge 1 commit into
mainfrom
05-22-catch_more_specific_errors

Conversation

@kiftio
Copy link
Copy Markdown
Contributor

@kiftio kiftio commented May 22, 2026

What changes are you making?

Replaces broad catch (e: Exception) blocks with specific exception types (SerializationException, NoSuchElementException, SecurityException) across the Android platform to avoid silently swallowing unexpected exceptions.

  • CheckoutBridge: Catches SerializationException and NoSuchElementException separately, extracting the error notification logic into a shared notifyDecodeFailure helper.
  • CheckoutProtocol: Narrows exception handling in error, windowOpen, and checkout notification decoders, and refactors Client.process to extract decodeRequest with a scoped SerializationException catch. Replaces runCatching with an explicit try/catch in Typed.dispatch.
  • EmbeddedCheckoutProtocol: Replaces unsafe jsonObject, jsonArray, and jsonPrimitive extension property accesses (which throw IllegalArgumentException) with safe as? casts that throw SerializationException on invalid structure, ensuring malformed ec.ready params are handled as parse errors.
  • ExternalUriLauncher: Narrows the fallback catch to SecurityException only.
  • CheckoutErrorDecoder and CheckoutCompletedEventDecoder: Replace catch (e: Exception) with SerializationException (and NoSuchElementException where applicable).

A test is added to verify that an empty error payload body triggers onCheckoutViewFailedWithError, and two new tests confirm that non-object ec.ready params and a non-array delegate field both produce a parse error response. A new ExternalUriLauncherTest verifies that a SecurityException from startActivity results in a Rejected result.

How to test

  1. Run the Android unit test suite: ./gradlew :lib:test
  2. Verify the new tests pass:
    • CheckoutBridgeTestshould call onCheckoutViewFailedWithError if error payload is empty
    • EmbeddedCheckoutProtocolTestec ready with non-object params sends parse error and ec ready with non-array delegate sends parse error
    • ExternalUriLauncherTestlaunch rejects when startActivity throws security exception
  3. Confirm no regressions in existing checkout flow tests.

Before you merge

Important

  • I've added tests to support my implementation
  • I have read and agree with the Contribution Guidelines
  • I have read and agree with the Code of Conduct
  • I've updated the relevant platform README (platforms/swift/README.md and/or platforms/android/README.md)

Releasing a new Swift version?
  • I have bumped the version in ShopifyCheckoutKit.podspec
  • I have bumped the version in platforms/swift/Sources/ShopifyCheckoutKit/ShopifyCheckoutKit.swift
  • I have updated platforms/swift/CHANGELOG.md
  • I have updated the SwiftPM/CocoaPods version snippets in platforms/swift/README.md (major version only)
Releasing a new Android version?
  • I have bumped the versionName in platforms/android/lib/build.gradle
  • I have updated platforms/android/CHANGELOG.md
  • I have updated the Gradle/Maven version snippets in platforms/android/README.md

Tip

See the Contributing documentation for the full release process per platform.

Copy link
Copy Markdown
Contributor Author

kiftio commented May 22, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 22, 2026

React Native — Coverage Report

Lines Statements Branches Functions
Coverage: 98%
96.76% (239/247) 91.02% (142/156) 100% (64/64)

@kiftio kiftio force-pushed the 05-22-catch_more_specific_errors branch from f944ed4 to 64de28d Compare May 22, 2026 11:13
@kiftio kiftio marked this pull request as ready for review May 22, 2026 11:13
@kiftio kiftio requested a review from a team as a code owner May 22, 2026 11:14
Copy link
Copy Markdown
Contributor

@Juanita-Dash Juanita-Dash left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants