Skip to content
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

How to close websocket connection? #2

Closed
svirmi opened this issue Dec 24, 2023 · 2 comments · Fixed by #3
Closed

How to close websocket connection? #2

svirmi opened this issue Dec 24, 2023 · 2 comments · Fixed by #3
Assignees

Comments

@svirmi
Copy link

svirmi commented Dec 24, 2023

Hello Linsto!
Thank you for such an interesting library!
Can you please advice how to gracefully close websocket connection and to gracefully exit?

I did it like below, but not sure if it is the right way:

ctx, cancel := context.WithCancel(context.Background())

        defer cancel()

	obClient, err := bnWsMarket.NewSpotMarketStreamClient(ctx, &bnWsMarket.SpotMarketStreamCfg{
		Debug:   true,
		Logger:  slog.Default(),
		BaseURL: bnWsMarket.SpotMarketStreamBaseURL,
	})

// adding topics and listeners

// some other code

// and at the end of the program

obClient.UnSubscribe([]string{topic1, topic2})

So I guess it should probably close WS connection via context?

Thank you!

@linstohu
Copy link
Owner

linstohu commented Dec 25, 2023

Hi, @svirmi

Thank you for your encouragement, your support makes me feel that those times were not in vain.

This is a very good question, and your understanding is correct.

Regarding graceful close, my understanding is that you want to release all occupied resources, right? I think this should be automatically handled by the SpotMarketStreamClient within the package upon receiving the close command, without the need for explicit calls.

So, the question of 'graceful close' is upgraded to the question of 'who will pass the close signal.' As the saying goes: Do not communicate by sharing memory; instead, share memory by communicating.

Therefore, just use context.

@linstohu
Copy link
Owner

Well, I thought about it for a while, and then, I feel your question is quite representative; the question you raised is indeed a very good one.

From the caller's perspective, something like the following might be more user-friendly:

	cli, err := bnMarketWs.NewSpotMarketStreamClient(&bnMarketWs.SpotMarketStreamCfg{
		Debug:   true,
		Logger:  slog.Default(),
		BaseURL: bnMarketWs.SpotMarketStreamBaseURL,
	})
	if err != nil {
		return nil, err
	}

	err = cli.Open()
	if err != nil {
		return nil, err
	}

	// adding topics and listeners

	// some other code

	err = cli.Close()
	if err != nil {
		return nil, err
	}

I will consider adopting this approach in subsequent iterations.

@linstohu linstohu self-assigned this Dec 25, 2023
@linstohu linstohu linked a pull request Dec 26, 2023 that will close this issue
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 a pull request may close this issue.

2 participants