-
Notifications
You must be signed in to change notification settings - Fork 68
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
Switch handlers to use asynchronous iterables #430
Conversation
servers.describeServers( | ||
["connect-go (h2)", "@bufbuild/connect-node (h2c)"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This enables the tests added in #350 to run against @bufbuild/connect-node.
The diff for those files is larger than the actual change, because indentation changed, and the test helper http2Request()
was updated to take an object argument for readability.
The only actual change here was in the error message expectations, and adding a Content-Type to requests with an unsupported HTTP verb because connect-go and connect-node slightly differ in implementation in this case ( connect-node always returns 415 on Content-Type mismatch, regardless of verb).
connect: true, | ||
grpcWeb: false, | ||
grpc: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR changes the public API for creating handlers. The functions createXProtocol()
are no longer available.
All protocols are still enabled by default. You can disable individual protocols with a boolean property.
/** | ||
* @deprecated use compressionValidateOptions from @bufbuild/connect-core | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecated a couple of functions that are still used in transports. They will be replaced when we switch the Node transports to use asynchronous iterables.
/** | ||
* Convert a universal handler to a Node.js handler function. | ||
*/ | ||
export function universalHandlerToNodeHandler( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For handlers, this is now the single bridge between Node.js and our implementation.
return function handle(req: UniversalRequest): UniversalResponse { | ||
const type = parseContentType(req.header.get(headerContentType)); | ||
if (type == undefined || type.text) { | ||
return uResponseUnsupportedMediaType; | ||
} | ||
if (req.method !== methodPost) { | ||
return uResponseMethodNotAllowed; | ||
} | ||
const context = createHandlerContext( | ||
spec, | ||
req.header, | ||
{ | ||
[headerContentType]: type.binary ? contentTypeProto : contentTypeJson, | ||
}, | ||
{ | ||
[headerGrpcStatus]: grpcStatusOk, | ||
} | ||
); | ||
const compression = compressionNegotiate( | ||
opt.acceptCompression, | ||
req.header.get(headerEncoding), | ||
req.header.get(headerAcceptEncoding), | ||
headerAcceptEncoding | ||
); | ||
if (compression.response) { | ||
context.responseHeader.set(headerEncoding, compression.response.name); | ||
} | ||
const outputIt = pipe( | ||
req.body, | ||
transformPrepend<Uint8Array>(() => { | ||
// raise compression error to serialize it as a trailer status | ||
if (compression.error) throw compression.error; | ||
return undefined; | ||
}), | ||
transformSplitEnvelope(opt.readMaxBytes), | ||
transformDecompressEnvelope(compression.request, opt.readMaxBytes), | ||
transformParseEnvelope( | ||
serialization.getI(type.binary), | ||
trailerFlag, | ||
null | ||
), | ||
transformInvokeImplementation<I, O>(spec, context), | ||
transformSerializeEnvelope( | ||
serialization.getO(type.binary), | ||
opt.writeMaxBytes | ||
), | ||
transformCatchFinally<EnvelopedMessage>((e) => { | ||
const trailer = context.responseTrailer; | ||
if (e !== undefined) { | ||
if (e instanceof ConnectError) { | ||
setTrailerStatus(trailer, e); | ||
} else { | ||
setTrailerStatus( | ||
trailer, | ||
new ConnectError( | ||
"internal error", | ||
Code.Internal, | ||
undefined, | ||
undefined, | ||
e | ||
) | ||
); | ||
} | ||
} | ||
return { | ||
flags: trailerFlag, | ||
data: trailerSerialization.serialize(trailer), | ||
}; | ||
}), | ||
transformCompressEnvelope(compression.response, opt.compressMinBytes), | ||
transformJoinEnvelopes() | ||
); | ||
return { | ||
...uResponseOk, | ||
body: outputIt, | ||
header: context.responseHeader, | ||
}; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the implementation of gRPC-web handlers and shows the meat of this PR. Granted, we are shifting a lot of the complexity into connect-core, but the result seems more manageable than what we previously had.
No description provided.