Skip to content

Conversation

@suthar26
Copy link
Contributor

@suthar26 suthar26 commented Oct 17, 2025

  • adds explicit exception handling which was causing app crashes due to a race condition

@suthar26 suthar26 requested a review from a team as a code owner October 17, 2025 15:50
Copilot AI review requested due to automatic review settings October 17, 2025 15:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a safety wrapper around BackgroundEventSource to prevent crashes caused by RejectedExecutionException during start/close lifecycle. Key changes include introducing SafeBackgroundEventSource with guarded start/close methods and updating DevCycleClient to use the new wrapper.

  • Introduces SafeBackgroundEventSource with explicit exception handling
  • Replaces direct BackgroundEventSource usage in DevCycleClient with the safe wrapper

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
SafeBackgroundEventSource.kt New wrapper adds guarded start/close to avoid executor rejection crashes
DevCycleClient.kt Replaces raw BackgroundEventSource with SafeBackgroundEventSource to apply safer lifecycle management

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@suthar26 suthar26 marked this pull request as draft October 17, 2025 15:53
Copy link
Contributor

@jsalaber jsalaber left a comment

Choose a reason for hiding this comment

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

https://www.javadoc.io/doc/com.launchdarkly/okhttp-eventsource/latest/com/launchdarkly/eventsource/EventSource.Builder.html

I'm wondering if we can also make use of the options in the EventSource to always continue instead of throwing an exception

@suthar26 suthar26 force-pushed the parthsuthar/add-catch-for-exception branch from 3ba64bf to 3b233c4 Compare October 21, 2025 20:51
@suthar26 suthar26 marked this pull request as ready for review October 21, 2025 20:52
Copilot AI review requested due to automatic review settings October 21, 2025 20:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

ConnectStrategy.http(URI(config?.sse?.url))
.readTimeout(EVENT_SOURCE_RETRY_DELAY_MIN, TimeUnit.MINUTES)
)
).errorStrategy(ErrorStrategy.alwaysContinue())
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

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

Using ErrorStrategy.alwaysContinue() silently suppresses all errors, which could mask critical issues and make debugging difficult. Consider implementing a custom error strategy that logs errors before continuing, or use a strategy that retries with backoff for transient errors while failing fast for non-recoverable ones.

Suggested change
).errorStrategy(ErrorStrategy.alwaysContinue())
).errorStrategy { error, _ ->
DevCycleLogger.e("EventSource error: ${error.message}", error)
ErrorStrategy.Action.CONTINUE
}

Copilot uses AI. Check for mistakes.
@suthar26 suthar26 merged commit b6794ab into main Oct 21, 2025
5 checks passed
@suthar26 suthar26 deleted the parthsuthar/add-catch-for-exception branch October 21, 2025 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants