-
Notifications
You must be signed in to change notification settings - Fork 5
chore: switch identifyUser to return InvalidUser error in callback #239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 removes the throwing behavior from identifyUser
and switches to reporting ClientError.InvalidUser
via the callback, aligning it with how other errors are handled.
- Updated the synchronous
identifyUser
to call the callback with.InvalidUser
instead of throwing. - Refactored the async wrapper to use the callback-based API in a throwing continuation.
- Added new unit tests to verify error behavior for invalid users in both callback and async forms.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
DevCycle/DevCycleClient.swift | Changed identifyUser to non-throwing, invoke error callback on invalid input, and refactored async |
DevCycleTests/Models/DevCycleClientTests.swift | Added tests for identifyUser errors when given an empty user ID in both callback and async cases |
Comments suppressed due to low confidence (2)
DevCycle/DevCycleClient.swift:426
- The method signature was updated to remove
throws
but any existing doc comments or API documentation should also be updated to reflect this change and indicate that errors are now delivered via the callback.
public func identifyUser(user: DevCycleUser, callback: IdentifyCompletedHandler? = nil) {
DevCycleTests/Models/DevCycleClientTests.swift:804
- The async test creates an
XCTestExpectation
but never waits for it, which may cause the test to complete before assertions are executed. Either remove the expectation and assert directly in the async context or add await(for:timeout:)
(orawait fulfillment(of:timeout:)
) to ensure the expectation is fulfilled.
func testAsyncIdentifyUserWithInvalidUserCallsCallbackWithError() async throws {
ugh @jsalaber now that change means this is an issue: Which is probably a lot of folks are using |
throws
fromidentifyUser
, and returnClientError.InvalidUser
in error callback like the config errors