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

Cannot create pull stream / missing method #65

Closed
andig opened this issue Sep 18, 2021 · 6 comments
Closed

Cannot create pull stream / missing method #65

andig opened this issue Sep 18, 2021 · 6 comments

Comments

@andig
Copy link
Contributor

andig commented Sep 18, 2021

I'm trying to rebuild this example using the go client: https://developer.easee.cloud/page/signalr-code-examples

I'm assuming (I'm new to to SignalR) that on is synonym with PullStream?

This is what I have:

client, err := signalr.NewClient(ctx, conn)
if err != nil {
	return c, err
}

if err := client.Start(); err != nil {
	return c, err
}

if err := <-client.Send("SubscribeWithCurrentState", c.charger, true); err != nil {
	return c, err
}

go func() {
	productUpdateChan := client.PullStream("ProductUpdate")
	for res := range productUpdateChan {
		if res.Error != nil {
			c.log.ERROR.Println("ProductUpdate", res.Error)
		} else {
			c.log.TRACE.Printf("ProductUpdate %s", res.Value)
		}
	}
}()

Unfortunately, this always errors:

class=Client connection=Dbd8yRimrV-ORMlmZj6SLQ level=debug caller=loop.go:186 event="message received" message="signalr.invocationMessage{Type:1, Target:\"ProductUpdate\", InvocationID:\"\", Arguments:[]interface {}{json.RawMessage{0x7b, 0x22, 0x6d, 0x69, 0x64, 0x22, 0x3a, 0x22, 0x45, 0x48, 0x36, 0x33, 0x35, 0x38, 0x37, 0x30, 0x22, 0x2c, 0x22, 0x64, 0x61, 0x74, 0x61, 0x54, 0x79, 0x70, 0x65, 0x22, 0x3a, 0x32, 0x2c, 0x22, 0x69, 0x64, 0x22, 0x3a, 0x31, 0x36, 0x2c, 0x22, 0x74, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x22, 0x3a, 0x22, 0x32, 0x30, 0x32, 0x31, 0x2d, 0x30, 0x39, 0x2d, 0x31, 0x37, 0x54, 0x30, 0x36, 0x3a, 0x30, 0x36, 0x3a, 0x34, 0x32, 0x5a, 0x22, 0x2c, 0x22, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x22, 0x3a, 0x22, 0x31, 0x22, 0x7d}}, StreamIds:[]string(nil)}"
class=Client connection=Dbd8yRimrV-ORMlmZj6SLQ level=info event=getMethod error="missing method" name=ProductUpdate reaction="send completion with error"

It seems I have no way getting around the missing method error?

@andig
Copy link
Contributor Author

andig commented Sep 18, 2021

I also have the suspicion, that the code might be racy: if the server sends data before the client.PullStream has been created these will error. Creating the pull stream before starting the client however panics.

@andig
Copy link
Contributor Author

andig commented Sep 18, 2021

Oh, got it. Shouldn't use streams put public methods.

@andig andig closed this as completed Sep 18, 2021
@philippseith
Copy link
Owner

PullStream pulls a stream from the server. It sends a StreamingInvocation to the server. Maybe the name is missleading. See https://github.com/dotnet/aspnetcore/blob/main/src/SignalR/docs/specs/HubProtocol.md#streaming.
The example server does publish its values by calling methods on the server (callbacks, therefore the javascript client is has this 'on' method). But as I wrote here #63 (comment), calling methods on the server and the client is barely the same thing, so I decided to implement calling callbacks like calling hub methods, aka as calling public methods of the hub (server) or receiver (client)

@philippseith
Copy link
Owner

FYI streaming has less protocol overhead than calling the client for every new item.

@andig
Copy link
Contributor Author

andig commented Sep 19, 2021

FYI streaming has less protocol overhead than calling the client for every new item.

Not my choice as client…

As I‘m slowly beginning to understand how signalR actually works, I‘m not entirely happy with the API exposed. The extensive use of reflection make it hard to type-check anything at compile time. Exposing the handlers as public methods on the receiver is not self-explanatory. If you should ever consider a 2.0 version it might be nice to make the handlers explicitly configurable per function, maybe something like

f := func(param …interface{}) { … }
party.Handler(„ProductUpdate“, f)

@philippseith
Copy link
Owner

IMHO, type checking is not missing because of the usage of reflection, but because SignalR has no IDL like gPRC or https://github.com/twitchtv/twirp.
Reflection is used by the archetypical .net SignalR server and has been brought into the go server by David Fowler himself when he started this project.
But you are right: Adding a handler interface is a simple (and simply understood) alternative to the current strategy.

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

No branches or pull requests

2 participants