Skip to content

Improved fallback mechanism when user is not using hilt/dagger #43

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

Merged
merged 5 commits into from
Feb 3, 2025

Conversation

mrajatttt
Copy link
Contributor

Issue Number:

Description:

What are the changes? Why are we making them?

  • We got feedback that app was crashing if user did not have dagger/hilt dependencies in their app, now we have improved the fallback and it will try to initialize manually.

Functional backward compatibility:

Does this change introduce backwards incompatible changes? [YES/NO]

Does this change introduce any new dependency? [YES/NO]


Testing:

Is the code unit tested?

Have you tested the changes with a sample UI (e.g. Android Mobile Chat Example)?

List manual testing steps:

  • Add Steps below:

Here are a list of manual test cases to run through:

  • Initiating chat and connecting with an agent
  • Retrieving transcript
  • Disconnecting from chat
  • Sending a message to the agent
    • See typing bubbles on agent side
    • See read/delivered receipt on client side
    • Receiving a message from the agent
    • See typing bubbles on client side
    • See read/delivered receipt on agent side
    • Sending an attachment to the agent (try .txt, .pdf, .jpg)
    • Preview the attachment on click
    • Receiving an attachment from the agent
    • Preview the attachment on click
  • Close the application (Without ending chat) → open app again → Start chat → Should Retrieve transcript from a previous chat session

@mrajatttt mrajatttt requested review from agarwhi and mliao95 January 28, 2025 21:59
true
} catch (e: ClassNotFoundException) {
SDKLogger.logger.logDebug {"Hilt is not available"}
SDKLogger.logger.logError { "Hilt is not available: ${e.message}" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to log this as an error or as a debug? I feel like it is almost expected that users will fall into this path and we shouldn't emit errors for expected workflows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. will update it

@mrajatttt mrajatttt force-pushed the rajatttt/better-hilt-support branch from ea85776 to ef5ccdf Compare January 28, 2025 22:50
@mliao95 mliao95 self-requested a review January 28, 2025 23:39
mliao95
mliao95 previously approved these changes Jan 29, 2025
agarwhi
agarwhi previously approved these changes Jan 29, 2025
@mrajatttt mrajatttt dismissed stale reviews from agarwhi and mliao95 via 45295d9 February 3, 2025 18:59
@mrajatttt mrajatttt requested review from agarwhi and mliao95 February 3, 2025 19:23
false
} catch (e: Exception) {
// Catch any unexpected issues during class detection
SDKLogger.logger.logDebug { "Error while checking Hilt availability: ${e.message}" }
Copy link

Choose a reason for hiding this comment

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

why this is logged as Debug not err ?

}
} catch (e: Exception) {
// Handle unexpected errors during initialization
SDKLogger.logger.logDebug { "Error initializing ChatSession: ${e.message}" }
Copy link

Choose a reason for hiding this comment

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

Is this as debug statement correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we aim to print it as a debug which won't interrupt the flow

@mrajatttt mrajatttt merged commit 18274c7 into main Feb 3, 2025
4 checks passed
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