Skip to content

@W-22699714: [Android] Improve error handling at code exchange#2932

Draft
JohnsonEricAtSalesforce wants to merge 2 commits into
forcedotcom:devfrom
JohnsonEricAtSalesforce:feature/w-22699714_android-improve-error-handling-at-code-exchange
Draft

@W-22699714: [Android] Improve error handling at code exchange#2932
JohnsonEricAtSalesforce wants to merge 2 commits into
forcedotcom:devfrom
JohnsonEricAtSalesforce:feature/w-22699714_android-improve-error-handling-at-code-exchange

Conversation

@JohnsonEricAtSalesforce

Copy link
Copy Markdown
Contributor

Summary

  • Add user-friendly "app blocked" toast when code exchange fails with client_blocked error
  • Follow existing isLightningTokenEndpointFailure pattern in LoginActivity.onAuthFlowError()
  • Add sf__app_blocked_error string resource
  • Add 3 doCodeExchange error path tests in LoginViewModelMockTest
  • Add 3 onAuthFlowError integration tests in LoginActivityAuthErrorTest

Test plan

  • LoginViewModelMockTest.doCodeExchange_whenExchangeCodeThrowsClientBlocked_callsOnAuthFlowError passes
  • LoginViewModelMockTest.doCodeExchange_whenExchangeCodeThrowsIOException_callsOnAuthFlowError passes
  • LoginViewModelMockTest.doCodeExchange_whenExchangeCodeThrowsOAuthFailed_neverCallsOnAuthFlowComplete passes
  • LoginActivityAuthErrorTest.onAuthFlowError_givenClientBlocked_broadcastsWithCorrectExtras passes
  • LoginActivityAuthErrorTest.onAuthFlowError_givenClientBlocked_showsAppBlockedToast passes
  • LoginActivityAuthErrorTest.onAuthFlowError_givenGenericOAuthError_broadcastsWithCorrectExtras passes
  • Lint passes with no new warnings
  • CI green on PR

Add user-friendly 'app blocked' toast when code exchange fails with
client_blocked error. Follows same pattern as existing Lightning URL
error handling in LoginActivity.onAuthFlowError().

- Add sf__app_blocked_error string resource
- Add isClientBlocked check + when expression for toast message
- Add 3 doCodeExchange error path tests in LoginViewModelMockTest
- Add 3 onAuthFlowError integration tests in LoginActivityAuthErrorTest
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
1 Warning
⚠️ libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/LoginActivity.kt#L680 - Switch statement on an int with known associated constant missing case BiometricManager.BIOMETRIC_ERROR_IDENTITY_CHECK_NOT_ACTIVE

Generated by 🚫 Danger

TokenErrorResponse.error and .errorDescription are public Java fields,
not methods. MockK's every { } block only intercepts method calls, so
using every { mock.error } returns value triggers "Missing mocked calls
inside every { ... } block". Fix by directly assigning the fields on
the relaxed mock instance instead.
<string name="sf__managed_app_error">Authentication only allowed from managed device.</string>
<string name="sf__jwt_authentication_error">JWT authentication error. Please try again.</string>
<string name="sf__lightning_url_code_exchange_error">Lightning URLs are not supported for OAuth code exchange. Use your My Domain URL instead.</string>
<string name="sf__app_blocked_error">This app has been blocked. Contact your administrator for assistance.</string>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The server returns user_blocked error when the app failed app attestation.
The error returned will probably change (soon) to reflect that - but our message here could already be more descriptive.

)

viewModel.clearCookies()
val isClientBlocked = e is OAuthFailedException

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's also recognize CLIENT_BLOCKED_RETRY_ERROR.
For now we will just bubble it up to the app/user.
Later, we might add retry-logic (in the case of refresh flow).

@wmathurin

wmathurin commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

(For a new story / PR) should we revisit our oauth error parsing to include all server errors (see server enum here) ??

Ideally the user should never just see the almost white web page with a cryptic error string.

@brandon-page @bbirman thoughts?

@JohnsonEricAtSalesforce JohnsonEricAtSalesforce marked this pull request as ready for review June 18, 2026 00:08
@JohnsonEricAtSalesforce JohnsonEricAtSalesforce marked this pull request as draft June 18, 2026 00:10
@bbirman

bbirman commented Jun 18, 2026

Copy link
Copy Markdown
Member

(For a new story / PR) should we revisit our oauth error parsing to include all server errors (see server enum here) ??

Ideally the user should never just see the almost white web page with a cryptic error string.

@brandon-page @bbirman thoughts?

yes please!

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.

3 participants