Skip to content

Commit

Permalink
Move feedback into ClientCompatResponse (#776)
Browse files Browse the repository at this point in the history 
This moves the `feedback` field into the `ClientCompatResponse` so that
it is easier to report feedback on the client-side. This way the reference
client can simply set the `Feedback` field when returning a response
result, which will then be written out by the server runner.
  • Loading branch information
@smaye81
smaye81 committed Feb 7, 2024
1 parent f5edb25 commit bbda232
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 135 deletions.
12 changes: 6 additions & 6 deletions grpcwebclient/gen/proto/connectrpc/conformance/v1/client_compat_pb.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,6 @@ export class ClientCompatResponse extends jspb.Message {
hasError(): boolean;
clearError(): ClientCompatResponse;

getFeedbackList(): Array<string>;
setFeedbackList(value: Array<string>): ClientCompatResponse;
clearFeedbackList(): ClientCompatResponse;
addFeedback(value: string, index?: number): ClientCompatResponse;

getResultCase(): ClientCompatResponse.ResultCase;

serializeBinary(): Uint8Array;
Expand All @@ -233,7 +228,6 @@ export namespace ClientCompatResponse {
testName: string,
response?: ClientResponseResult.AsObject,
error?: ClientErrorResult.AsObject,
feedbackList: Array<string>,
}

export enum ResultCase {
Expand Down Expand Up @@ -272,6 +266,11 @@ export class ClientResponseResult extends jspb.Message {
hasWireDetails(): boolean;
clearWireDetails(): ClientResponseResult;

getFeedbackList(): Array<string>;
setFeedbackList(value: Array<string>): ClientResponseResult;
clearFeedbackList(): ClientResponseResult;
addFeedback(value: string, index?: number): ClientResponseResult;

serializeBinary(): Uint8Array;
toObject(includeInstance?: boolean): ClientResponseResult.AsObject;
static toObject(includeInstance: boolean, msg: ClientResponseResult): ClientResponseResult.AsObject;
Expand All @@ -288,6 +287,7 @@ export namespace ClientResponseResult {
responseTrailersList: Array<connectrpc_conformance_v1_service_pb.Header.AsObject>,
numUnsentRequests: number,
wireDetails?: WireDetails.AsObject,
feedbackList: Array<string>,
}
}

Expand Down
113 changes: 53 additions & 60 deletions grpcwebclient/gen/proto/connectrpc/conformance/v1/client_compat_pb.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ if (goog.DEBUG && !COMPILED) {
* @constructor
*/
proto.connectrpc.conformance.v1.ClientCompatResponse = function(opt_data) {
jspb.Message.initialize(this, opt_data, 0, -1, proto.connectrpc.conformance.v1.ClientCompatResponse.repeatedFields_, proto.connectrpc.conformance.v1.ClientCompatResponse.oneofGroups_);
jspb.Message.initialize(this, opt_data, 0, -1, null, proto.connectrpc.conformance.v1.ClientCompatResponse.oneofGroups_);
};
goog.inherits(proto.connectrpc.conformance.v1.ClientCompatResponse, jspb.Message);
if (goog.DEBUG && !COMPILED) {
Expand Down Expand Up @@ -1542,13 +1542,6 @@ proto.connectrpc.conformance.v1.ClientCompatRequest.prototype.hasRawRequest = fu



/**
* List of repeated fields within this message type.
* @private {!Array<number>}
* @const
*/
proto.connectrpc.conformance.v1.ClientCompatResponse.repeatedFields_ = [4];

/**
* Oneof group definitions for this message. Each group defines the field
* numbers belonging to that group. When of these fields' value is set, all
Expand Down Expand Up @@ -1608,8 +1601,7 @@ proto.connectrpc.conformance.v1.ClientCompatResponse.toObject = function(include
var f, obj = {
testName: jspb.Message.getFieldWithDefault(msg, 1, ""),
response: (f = msg.getResponse()) && proto.connectrpc.conformance.v1.ClientResponseResult.toObject(includeInstance, f),
error: (f = msg.getError()) && proto.connectrpc.conformance.v1.ClientErrorResult.toObject(includeInstance, f),
feedbackList: (f = jspb.Message.getRepeatedField(msg, 4)) == null ? undefined : f
error: (f = msg.getError()) && proto.connectrpc.conformance.v1.ClientErrorResult.toObject(includeInstance, f)
};

if (includeInstance) {
Expand Down Expand Up @@ -1660,10 +1652,6 @@ proto.connectrpc.conformance.v1.ClientCompatResponse.deserializeBinaryFromReader
reader.readMessage(value,proto.connectrpc.conformance.v1.ClientErrorResult.deserializeBinaryFromReader);
msg.setError(value);
break;
case 4:
var value = /** @type {string} */ (reader.readString());
msg.addFeedback(value);
break;
default:
reader.skipField();
break;
Expand Down Expand Up @@ -1716,13 +1704,6 @@ proto.connectrpc.conformance.v1.ClientCompatResponse.serializeBinaryToWriter = f
proto.connectrpc.conformance.v1.ClientErrorResult.serializeBinaryToWriter
);
}
f = message.getFeedbackList();
if (f.length > 0) {
writer.writeRepeatedString(
4,
f
);
}
};


Expand Down Expand Up @@ -1818,50 +1799,13 @@ proto.connectrpc.conformance.v1.ClientCompatResponse.prototype.hasError = functi
};


/**
* repeated string feedback = 4;
* @return {!Array<string>}
*/
proto.connectrpc.conformance.v1.ClientCompatResponse.prototype.getFeedbackList = function() {
return /** @type {!Array<string>} */ (jspb.Message.getRepeatedField(this, 4));
};


/**
* @param {!Array<string>} value
* @return {!proto.connectrpc.conformance.v1.ClientCompatResponse} returns this
*/
proto.connectrpc.conformance.v1.ClientCompatResponse.prototype.setFeedbackList = function(value) {
return jspb.Message.setField(this, 4, value || []);
};


/**
* @param {string} value
* @param {number=} opt_index
* @return {!proto.connectrpc.conformance.v1.ClientCompatResponse} returns this
*/
proto.connectrpc.conformance.v1.ClientCompatResponse.prototype.addFeedback = function(value, opt_index) {
return jspb.Message.addToRepeatedField(this, 4, value, opt_index);
};


/**
* Clears the list making it empty but non-null.
* @return {!proto.connectrpc.conformance.v1.ClientCompatResponse} returns this
*/
proto.connectrpc.conformance.v1.ClientCompatResponse.prototype.clearFeedbackList = function() {
return this.setFeedbackList([]);
};



/**
* List of repeated fields within this message type.
* @private {!Array<number>}
* @const
*/
proto.connectrpc.conformance.v1.ClientResponseResult.repeatedFields_ = [1,2,4];
proto.connectrpc.conformance.v1.ClientResponseResult.repeatedFields_ = [1,2,4,7];



Expand Down Expand Up @@ -1902,7 +1846,8 @@ proto.connectrpc.conformance.v1.ClientResponseResult.toObject = function(include
responseTrailersList: jspb.Message.toObjectList(msg.getResponseTrailersList(),
connectrpc_conformance_v1_service_pb.Header.toObject, includeInstance),
numUnsentRequests: jspb.Message.getFieldWithDefault(msg, 5, 0),
wireDetails: (f = msg.getWireDetails()) && proto.connectrpc.conformance.v1.WireDetails.toObject(includeInstance, f)
wireDetails: (f = msg.getWireDetails()) && proto.connectrpc.conformance.v1.WireDetails.toObject(includeInstance, f),
feedbackList: (f = jspb.Message.getRepeatedField(msg, 7)) == null ? undefined : f
};

if (includeInstance) {
Expand Down Expand Up @@ -1968,6 +1913,10 @@ proto.connectrpc.conformance.v1.ClientResponseResult.deserializeBinaryFromReader
reader.readMessage(value,proto.connectrpc.conformance.v1.WireDetails.deserializeBinaryFromReader);
msg.setWireDetails(value);
break;
case 7:
var value = /** @type {string} */ (reader.readString());
msg.addFeedback(value);
break;
default:
reader.skipField();
break;
Expand Down Expand Up @@ -2044,6 +1993,13 @@ proto.connectrpc.conformance.v1.ClientResponseResult.serializeBinaryToWriter = f
proto.connectrpc.conformance.v1.WireDetails.serializeBinaryToWriter
);
}
f = message.getFeedbackList();
if (f.length > 0) {
writer.writeRepeatedString(
7,
f
);
}
};


Expand Down Expand Up @@ -2253,6 +2209,43 @@ proto.connectrpc.conformance.v1.ClientResponseResult.prototype.hasWireDetails =
};


/**
* repeated string feedback = 7;
* @return {!Array<string>}
*/
proto.connectrpc.conformance.v1.ClientResponseResult.prototype.getFeedbackList = function() {
return /** @type {!Array<string>} */ (jspb.Message.getRepeatedField(this, 7));
};


/**
* @param {!Array<string>} value
* @return {!proto.connectrpc.conformance.v1.ClientResponseResult} returns this
*/
proto.connectrpc.conformance.v1.ClientResponseResult.prototype.setFeedbackList = function(value) {
return jspb.Message.setField(this, 7, value || []);
};


/**
* @param {string} value
* @param {number=} opt_index
* @return {!proto.connectrpc.conformance.v1.ClientResponseResult} returns this
*/
proto.connectrpc.conformance.v1.ClientResponseResult.prototype.addFeedback = function(value, opt_index) {
return jspb.Message.addToRepeatedField(this, 7, value, opt_index);
};


/**
* Clears the list making it empty but non-null.
* @return {!proto.connectrpc.conformance.v1.ClientResponseResult} returns this
*/
proto.connectrpc.conformance.v1.ClientResponseResult.prototype.clearFeedbackList = function() {
return this.setFeedbackList([]);
};





Expand Down
4 changes: 2 additions & 2 deletions internal/app/connectconformance/server_runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@ func runTestCasesForServer(
default:
results.assert(name, expectations[resp.TestName], resp.GetResponse())
}
if isReferenceClient && resp != nil {
for _, msg := range resp.Feedback {
if isReferenceClient && resp.GetResponse() != nil {
for _, msg := range resp.GetResponse().Feedback {
results.recordSideband(resp.TestName, msg)
}
}
Expand Down
38 changes: 32 additions & 6 deletions internal/app/referenceclient/impl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,15 @@ func (i *invoker) unary(
}
}

wireDetails, feedback := getWireDetails(ctx)

return &v1.ClientResponseResult{
ResponseHeaders: headers,
ResponseTrailers: trailers,
Payloads: payloads,
Error: protoErr,
WireDetails: getWireDetails(ctx),
Feedback: feedback,
WireDetails: wireDetails,
}, nil
}

Expand Down Expand Up @@ -191,12 +194,15 @@ func (i *invoker) idempotentUnary(
trailers = internal.ConvertToProtoHeader(resp.Trailer())
}

wireDetails, feedback := getWireDetails(ctx)

return &v1.ClientResponseResult{
ResponseHeaders: headers,
ResponseTrailers: trailers,
Payloads: payloads,
Error: protoErr,
WireDetails: getWireDetails(ctx),
Feedback: feedback,
WireDetails: wireDetails,
}, nil
}

Expand Down Expand Up @@ -281,12 +287,15 @@ func (i *invoker) serverStream(
}
}

wireDetails, feedback := getWireDetails(ctx)

return &v1.ClientResponseResult{
ResponseHeaders: headers,
ResponseTrailers: trailers,
Payloads: payloads,
Error: protoErr,
WireDetails: getWireDetails(ctx),
Feedback: feedback,
WireDetails: wireDetails,
}, nil
}

Expand Down Expand Up @@ -351,12 +360,15 @@ func (i *invoker) clientStream(
trailers = internal.ConvertToProtoHeader(resp.Trailer())
}

wireDetails, feedback := getWireDetails(ctx)

return &v1.ClientResponseResult{
ResponseHeaders: headers,
ResponseTrailers: trailers,
Payloads: payloads,
Error: protoErr,
WireDetails: getWireDetails(ctx),
Feedback: feedback,
WireDetails: wireDetails,
}, nil
}

Expand All @@ -377,7 +389,7 @@ func (i *invoker) bidiStream(
return
}

result.WireDetails = getWireDetails(ctx)
result.WireDetails, result.Feedback = getWireDetails(ctx)

// Read headers and trailers from the stream
result.ResponseHeaders = internal.ConvertToProtoHeader(stream.ResponseHeader())
Expand Down Expand Up @@ -510,12 +522,26 @@ func (i *invoker) unimplemented(
// Invoke the Unary call
_, err := i.client.Unimplemented(ctx, request)

wireDetails, feedback := getWireDetails(ctx)

return &v1.ClientResponseResult{
Error: internal.ConvertErrorToProtoError(err),
WireDetails: getWireDetails(ctx),
Feedback: feedback,
WireDetails: wireDetails,
}, nil
}

// getWireDetails gets the wire details from the given context and returns any
// feedback errors that may been encountered.
func getWireDetails(ctx context.Context) (*v1.WireDetails, []string) {
var feedback []string
wireDetails, err := buildWireDetails(ctx)
if err != nil {
feedback = append(feedback, fmt.Sprintf("failed to compute response wire details: %v", err))
}
return wireDetails, feedback
}

// Creates a new invoker around a ConformanceServiceClient.
func newInvoker(transport http.RoundTripper, url *url.URL, opts []connect.ClientOption) *invoker {
opts = append(opts, connect.WithInterceptors(userAgentClientInterceptor{}))
Expand Down
Loading

0 comments on commit bbda232

Please sign in to comment.