-
Notifications
You must be signed in to change notification settings - Fork 230
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
[http-server-javascript] Support new-style multipart requests (and some other things) #5514
base: main
Are you sure you want to change the base?
[http-server-javascript] Support new-style multipart requests (and some other things) #5514
Conversation
All changed packages have been documented.
Show changes
|
You can try these changes here
|
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.
A few questions / comments, but this looks good
|
||
// prettier-ignore | ||
const lines = [ | ||
"// Copyright (c) Microsoft Corporation", |
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.
I wonder is we want this copyright in any generated code? There is an issue to remove this from generated c-sharp server code.
"import type * as http from \"node:http\";", | ||
"", | ||
"export interface HttpPart {", | ||
" headers: { [k: string]: string | undefined };", |
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.
I wonder if the name of the part should be left in the content-disposition header, or captured as a separate property for matching with the expected parts
This PR implements handling for new-style multipart requests and heavily refactors the multipart core to use web streams. While this doesn't enable streaming multipart yet (i.e. the implementation layer still buffers the parts before providing them to the business logic), it's a step towards streaming multipart.
TypeSpec.Http.HttpPart
(link [core] Add signifier that designates a "must-understand" type. #5275).bytes
throughNodeJS.Buffer
asUint8Array
.gensym
, which generates unique symbols containing a monotonically incrementing counter. This helps me prevent collisions between names of variables I generate and names of variables generated from user typespec code without requiring a full-blown scope solution. It costs readability of the generated code, but isn't so bad.http.IncomingMessage
into a ReadableStream, then the bytes read from the multipart body are split into parts and streamed individually. Finally, each individual part stream is consumed by a transform that reads enough of the stream to parse the headers and then passes the rest of the body through to a final "body stream."HttpContext
. The application logic can use these handlers to manually respond with validation/internal errors in whichever way the router would ordinarily respond to them.