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

fix(ClientRequest): support 100 continue flow #599

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mikicho
Copy link
Contributor

@mikicho mikicho commented Jul 6, 2024

@mikicho mikicho mentioned this pull request Jul 6, 2024
19 tasks
@HonzaMac
Copy link

For mine use case, this error occur when using localstack and calling S3 upload. Uploading for first byte take so long (timeouts), that localstack sends back 100. 100 is not handled by undici/native fetch in v20 correctly and it throws error of not handle status code by undici.

@kettanaito
Copy link
Member

kettanaito commented Sep 8, 2024

I've pushed the fix where we are now able to construct a 100 Continue response without it throwing (<199 are not user-configurable status codes).

Updates the tests, no idea why they are failing only when (1) the interceptor is on; (2) the request.end() is nested in the continue event callback. Sharing the log outputs for two scenarios (bypass and mocked)

Bypass

SOCKET WRITE POST /resource HTTP/1.1
expect: 100-continue
Host: 127.0.0.1:62344
Connection: close
Transfer-Encoding: chunked


SOCKET EMIT [ 'resume' ]

stdout | modules/http/compliance/http-request-continue.test.ts > emits "continue" event for a request with "100-continue" expect header
SOCKET EMIT [ 'connect' ]
SOCKET CONNECT
SOCKET EMIT [ 'ready' ]
!!![server] added req.on(data)
SOCKET PUSH HTTP/1.1 100 Continue


SOCKET EMIT [ 'data', 'HTTP/1.1 100 Continue\r\n\r\n' ]
REQ CONTINUE
REQ END
!!!! writing request...
SOCKET WRITE 5
SOCKET WRITE 

SOCKET WRITE hello
SOCKET WRITE 

SOCKET WRITE 0


REQ FINISH

[server] req data: hello

SOCKET PUSH HTTP/1.1 200 OK
X-Powered-By: Express
Date: Sun, 08 Sep 2024 14:59:48 GMT
Connection: close
Transfer-Encoding: chunked

5
hello
0


SOCKET EMIT [
  'data',
  'HTTP/1.1 200 OK\r\n' +
    'X-Powered-By: Express\r\n' +
    'Date: Sun, 08 Sep 2024 14:59:48 GMT\r\n' +
    'Connection: close\r\n' +
    'Transfer-Encoding: chunked\r\n' +
    '\r\n' +
    '5\r\n' +
    'hello\r\n' +
    '0\r\n' +
    '\r\n'
]
REQ RESPONSE

SOCKET FINISH

SOCKET EMIT [ 'prefinish' ]
SOCKET EMIT [ 'finish' ]
SOCKET EMIT [ 'close', false ]

SOCKET CLOSE

Mocked

SOCKET WRITE POST /resource HTTP/1.1
expect: 100-continue
Host: 127.0.0.1:62777
Connection: close
Transfer-Encoding: chunked


[*] request POST http://127.0.0.1:62777/resource
SOCKET EMIT [ 'resume' ]
SOCKET EMIT [ 'resume' ]
SOCKET EMIT [ 'connect' ]
SOCKET CONNECT
SOCKET EMIT [ 'ready' ]
!!![server] added req.on(data)

SOCKET PUSH HTTP/1.1 100 Continue


SOCKET EMIT [ 'data', 'HTTP/1.1 100 Continue\r\n\r\n' ]
REQ CONTINUE
REQ END
!!!! writing request...
SOCKET WRITE 5
SOCKET WRITE 

SOCKET WRITE hello
REQ BODY! hello true
SOCKET WRITE 

SOCKET WRITE 0


REQ FINISH

// HANGS HERE FOR A WHILE UNTIL TEST TIMESOUT
// BECAUSE "req.on(data)" NEVER EMITS, AND THUS
// THE SERVER NEVER SENDS A RESPONSE.

SOCKET END

SOCKET FINISH

SOCKET EMIT [ 'end' ]
SOCKET EMIT [ 'close' ]
SOCKET EMIT [ 'prefinish' ]
SOCKET EMIT [ 'finish' ]
SOCKET EMIT [ 'close', false ]

SOCKET CLOSE
SOCKET CLOSE

@kettanaito kettanaito added the help wanted Extra attention is needed label Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants