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

Introduce X-Webrpc-Client header to all generated clients #251

Open
VojtechVitek opened this issue Mar 4, 2024 · 1 comment
Open

Introduce X-Webrpc-Client header to all generated clients #251

VojtechVitek opened this issue Mar 4, 2024 · 1 comment

Comments

@VojtechVitek
Copy link
Contributor

VojtechVitek commented Mar 4, 2024

I propose to add a new X-Webrpc-Client header to all webrpc client requests:

X-Webrpc-Client: {serviceName}@{schemaVersion}; webrpc@{webrpcGenVersion}; github.com/webrpc/gen-golang@{templateVersion}

Example:

X-Webrpc-Client: indexer@v0.2.0; webrpc@v0.18.0; github.com/webrpc/gen-golang@v0.18.2

Potential use-cases from the server perspective:

  • Telemetry on client version distribution
  • Enables server to error out on "deprecated" version of the schema
  • Very helpful for debugging HTTP 400 errors (missing fields, renamed fields etc.)

Go server

Let's add a helper function to help parse the schema version easily.

func GetClientHeader(r *http.Request) (schema string, webrpc string, gen string) {
  // value of header or ""
}

func ParseVersion(r *http.Request) (major, minor, bugfix int) {
  // parse X-Webrpc-Client header and return the details of the schema version
  // returns 0, 0, 0 if header is missing
}
func (s *ExampleServiceRPC) GetUser(ctx context.Context, header map[string]string, userID uint64) (*User, error) {
	schema, webrpc, gen := proto.GetClientHeader(r)
	
	major, minor, _ := proto.ParseVersion(schema)
	if major <= 1 && minor <= 8 {
		return nil, proto.ErrUnsupportedClientVersion.WithCause(fmt.Errorf("the latest supported webrpc client version is 1.8.x"))
	}
	
	// ...
}

BREAKING CHANGE?

Adding a new header to all requests might lead to CORS errors unless the server whitelists the new header. So perhaps we should make this an opt-in feature for now. However, I'd greatly support making this field mandatory as part of "webrpc protocol standard" down the road. Maybe we can make this an opt-out feature after a few releases. Or maybe we can enforce it in the generated client initialization - a new required field when you instantiate the client. Thoughts?

@david-littlefarmer
Copy link

Two thoughts

First

I tried to look up the naming convention for headers, and it looks like, the use of X- prefix is not recommended anymore.
Check this https://www.rfc-editor.org/rfc/rfc6648
This can be ignored it and just use X-Webrpc-Client instead of Webrpc-Client.

Second

I would change the structure from this:
X-Webrpc-Client: {serviceName}@{schemaVersion}; webrpc@{webrpcGenVersion}; github.com/webrpc/gen-golang@{templateVersion}

To this:
X-Webrpc-Client: service={serviceName}@{schemaVersion}; webrpc-verison={webrpcGenVersion}; template={template-source}@{templateVersion}

So the change would be to make a key-val string, same as for Set-Cookie https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie

So for example:
X-Webrpc-Client: service=indexer@v0.2.0; webrpc-version=v0.18.0; template=github.com/webrpc/gen-golang@v0.18.2

===

And to combine both thoughts:
Webrpc-Client: service=indexer@v0.2.0; webrpc-version=v0.18.0; template=github.com/webrpc/gen-golang@v0.18.2

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