clientcompat issue with V8

[timostamm]

Thanks for this project!

I am the author of a protoc-plugin that supports Twirp clients.

Unfortunately, clientcompat started failing with the V8 release.

$ go get github.com/twitchtv/twirp/clientcompat
$ clientcompat -client clientcompat/client-runner.js
# some lines omitted, all PASS
Testing empty value... FAIL: client has wrong response, have  want 
Testing request value formatting... FAIL: client has wrong response, have v:1 want v:1
Testing handling invalid error formatting from server... PASS
FAILED with 2 failures, 19 successes

Exactly the same client passes with go get github.com/twitchtv/twirp/clientcompat@v7.0.0.

Between v7 and v8, my client is receiving the same request payload via stdin and is writing the same server response to stdout. This is true for both failed tests.

Any idea where this could be coming from?

[timostamm]

This appears to be a regression in clientcompat/main.go:

https://github.com/twitchtv/twirp/blob/fc964007b6a7badaa284114f13ebe08ce821354a/clientcompat/main.go#L164-L167

reflect.DeepEqual() does not work as intended with APIV2 because it compares the internal state which differs for messages with equal content.

PR see #313

[marioizquierdo]

Makes sense. Thanks for looking into this!

Could you share more information about how do you use cliencompact? I don't think our CI server is running it in our tests.

[timostamm]

clientcompat is used as described in your README. A make target runs (as part of CI and release build):

• compiles your clientcompat.proto using our plugin
• compiles our test client, which works just like your gocompat.go example implementation
• go gets your clientcompat
• runs your clientcompat against our test client

BTW, conformance testers like clientcompat are super helpful to implement against!

Yes, your CI does a go test but does not include your make target for clientcompat. If you run that manually you get a failure:

➜  twirp git:(master) ✗ make test_go_client
# Recompile and install generator
GOBIN="$PWD/bin" go install -v ./protoc-gen-twirp
GOBIN="$PWD/bin" go install -v ./protoc-gen-twirp_python
# Generate code from go:generate comments
go generate ./...
./build/clientcompat -client ./build/gocompat
Testing noop without error... PASS
Testing "canceled" error parsing... PASS
Testing "unknown" error parsing... PASS
Testing "invalid_argument" error parsing... PASS
Testing "deadline_exceeded" error parsing... PASS
Testing "not_found" error parsing... PASS
Testing "bad_route" error parsing... PASS
Testing "already_exists" error parsing... PASS
Testing "permission_denied" error parsing... PASS
Testing "unauthenticated" error parsing... PASS
Testing "resource_exhausted" error parsing... PASS
Testing "failed_precondition" error parsing... PASS
Testing "aborted" error parsing... PASS
Testing "out_of_range" error parsing... PASS
Testing "unimplemented" error parsing... PASS
Testing "internal" error parsing... PASS
Testing "unavailable" error parsing... PASS
Testing "data_loss" error parsing... PASS
Testing empty value... FAIL: client has wrong response, have  want 
Testing request value formatting... FAIL: client has wrong response, have v:1 want v:1
Testing handling invalid error formatting from server... PASS
FAILED with 2 failures, 19 successes
make: *** [test_go_client] Error 1

[gurre]

I recently hit issues with empty JSON values too. It seems like somewhere since v5.11.0 only the empty object {} is allowed in JSON requests otherwise resulting in a malformed: the json request could not be decoded-error.

These null request payloads came from a GraphQL integration having a GraphQL-schema with a Boolean response type. It was sending the JSON encoded string null in the request payload. The fix is to send an empty object {} instead. Just great having these encoding rules as part of the contract without even knowing it.