Skip to content

[#574] Return errors from AppAuth:ResumeExternalUserAgentFlowWithURL#955

Open
w-goog wants to merge 35 commits into
masterfrom
feature/add-resume-external-flow-errors
Open

[#574] Return errors from AppAuth:ResumeExternalUserAgentFlowWithURL#955
w-goog wants to merge 35 commits into
masterfrom
feature/add-resume-external-flow-errors

Conversation

@w-goog
Copy link
Copy Markdown
Collaborator

@w-goog w-goog commented Apr 28, 2026

Asserts are Bad Bad Not Good(TM). This PR deprecates the extant method and points developers to a new edition which returns errors rather than crashing with asserts.

Two cases are handled:

  1. When shouldHandleURL is false, the function returns NO and sets an error.
  2. When _pendingEndSessionCallback is nil, the function now returns an error.

w-goog added 30 commits April 28, 2026 13:34
…ithURL: shims

The shims forward to the new error-returning variant for backward compatibility; their callers see the deprecation, but the shim itself must compile cleanly under -Werror.
…ange

The deprecated resumeExternalUserAgentFlowWithURL: now silently returns NO
on invalid state instead of raising NSException. Spell that out so callers
relying on @try/@catch around the old API aren't surprised.
… API

The new SwiftUI/SPM sample (added in #952) was using the deprecated
resumeExternalUserAgentFlow(with:); update it to use the throwing variant,
mirroring the Carthage Swift sample.
Spell out both NO cases (URLMismatch + InvalidAuthorizationFlow) on the new
:error: method so callers know what NO can mean.
…acing string

OIDOAuthExceptionInvalidAuthorizationFlow is exception-prose, not error prose.
Use a clean inline localizedDescription at both call sites instead. The
constant remains in OIDError.{h,m} for backward compatibility.
…zationFlow

The previous wording ("may have already completed or was not started") was
speculative. Describe the actual trigger: the redirect URL was received but
no pending callback exists.
…ples

The previous snippets logged every error from resumeExternalUserAgentFlowWithURL:error:,
which is noisy because OIDErrorCodeURLMismatch fires on every unrelated deeplink.
Only log on OIDErrorCodeInvalidAuthorizationFlow.
@w-goog w-goog requested review from camden-king and mingyokim and removed request for camden-king May 4, 2026 21:34
@brnnmrls brnnmrls self-requested a review May 12, 2026 17:37
Comment thread CHANGELOG.md Outdated
Comment thread Sources/AppAuthCore/OIDError.h Outdated
Comment thread Sources/AppAuthCore/OIDAuthorizationService.m
Comment thread Sources/AppAuthCore/OIDAuthorizationService.m Outdated
Comment thread Sources/AppAuthCore/OIDAuthorizationService.m Outdated
Comment thread Sources/AppAuthCore/OIDAuthorizationService.m Outdated
Comment thread Sources/AppAuthCore/OIDAuthorizationService.m Outdated
Comment thread Sources/AppAuth/macOS/OIDRedirectHTTPHandler.m Outdated
Copy link
Copy Markdown
Collaborator

@brnnmrls brnnmrls left a comment

Choose a reason for hiding this comment

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

Logic looks pretty good to me. Left nits in this review. Will give this one more pass once comments are resolved!

w-goog added 4 commits May 13, 2026 14:23
…tional

Adding the new error-returning method as a required member of the
OIDExternalUserAgentSession protocol would break source compatibility
for any external conformer. Marking it @optional keeps the addition
backwards compatible so this change can ship as a minor version bump
rather than a major one. The companion @required directive preserves
the required status of failExternalUserAgentFlowWithError:.
@w-goog w-goog requested a review from brnnmrls May 13, 2026 23:57
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