Skip to content

Conversation

@rolodato
Copy link
Contributor

@rolodato rolodato commented Mar 26, 2025

Add WithPolling option

If WithRealtime was used, this also enables polling in the background. This lets us have polling as a fallback for SSE

Retry environment fetch on startup, don't panic in realtime

Instead of trying to fetch the environment document once and panicking from realtime if we couldn't, we now retry the initialisation indefinitely in a different goroutine. Once the environment initialises, we start the realtime connection as usual.

Retry SSE reconnections with exponential backoff + jitter

Used for error handling when SSE connection fails

Other features

  • Replace logger with slog
  • Added new WithSlogLogger option
  • Improved log messages
  • Add HTTP response and request logging

@rolodato rolodato changed the title feat: Add WithPollingEnabled option to poll while using real-time updates. Prevent real-time worker goroutine from panicking. Use slog internally, add WithSlogLogger option. Improve log messages feat: Add WithPolling option to poll while using real-time updates. Prevent real-time worker goroutine from panicking. Use slog internally, add WithSlogLogger option. Improve log messages Mar 26, 2025
* Don't panic in realtime
* Retry fetching environment forever if cannot initialise
* Use exponential backoff when reconnecting to SSE
@rolodato rolodato changed the title feat: Add WithPolling option to poll while using real-time updates. Prevent real-time worker goroutine from panicking. Use slog internally, add WithSlogLogger option. Improve log messages feat: Realtime improvements Mar 27, 2025
@rolodato rolodato marked this pull request as ready for review March 27, 2025 04:44
@rolodato rolodato requested a review from gagantrivedi March 27, 2025 04:45
client.go Outdated

c.log.Info("environment updated", "environment", env.APIKey)
c.once.Do(func() {
if c.config.useRealtime && c.realtime == nil {
Copy link
Member

Choose a reason for hiding this comment

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

This should not be part of this method; UpdateEnvironment doesn't need to know anything about real-time

for {
select {
case <-ticker.C:
if !pollForever {
Copy link
Member

Choose a reason for hiding this comment

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

This should be moved to real-time module/package; pollEnvironment should not be responsible for this

Copy link
Member

@gagantrivedi gagantrivedi left a comment

Choose a reason for hiding this comment

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

Approving this due to time constraints, but a complete refactor is needed.

@rolodato rolodato merged commit ac2e06e into main Mar 27, 2025
3 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