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

Nock compatibility #575

Open
16 of 19 tasks
mikicho opened this issue Jun 1, 2024 · 6 comments
Open
16 of 19 tasks

Nock compatibility #575

mikicho opened this issue Jun 1, 2024 · 6 comments

Comments

@mikicho
Copy link
Contributor

mikicho commented Jun 1, 2024

Transfer Encoding @mikicho

support transfer-encoding: chunked

#596

  • should pass filteringPath options
  • should pass filteringRequestBody options
  • match body with form multipart
  • records and replays objects correctly
  • records and replays correctly with filteringRequestBody
  • records and replays gzipped nocks correctly
  • records and replays the response body

preemtive timeout

We decided to remove this feature from Nock. It not relevant for fetch.
We should consider if/how we want to support this for httpClient. this is a nice feature.

  • emits a timeout - with setTimeout
  • emits a timeout - with options.timeout
  • emits a timeout - with Agent.timeout
  • emits a timeout - options.timeout takes precedence over Agent.timeout

Headers

  • folds duplicate headers the same as Node
  • when keys are duplicated, is evaluated once per input field name, in correct order

Recorder

  • does not record requests from previous sessions
  • logs recorded objects
@kettanaito
Copy link
Member

Thanks for putting this down, @mikicho! Let's split these between ourselves and finish the interceptor. Please, if you are working on a task, put your GitHub handle next to it so I'd know, and I will do the same.

As usual, we can start with adding a test for the thing, verifying if it's not passing already, and then opening pull requests to #515. Does that sound good to you?

@kettanaito
Copy link
Member

kettanaito commented Jun 1, 2024

socket emits connect and secureConnect

This should already be the case:

Not sure if we have tests for this, may be indirectly tested and still missing test cases just for the connect and secureConnect events.

Edit: I'm assigning this to myself, will ensure those events are emitted correctly.

@kettanaito
Copy link
Member

Added implementation for following redirect responses in fetch: #627.

@kettanaito
Copy link
Member

should be safe to call in the middle of a request

@mikicho, can you elaborate on this scenario, please? Nock itself works by patching http.get and friends, so the interceptor is applied before any requests happen. I think I'm just misunderstanding what "to call" refers to here, to be honest.

Do you mean a scenario like this?

http.get()

interceptor.on('request', ({ controller }) => controller.respondWith(new Response()))

@mikicho
Copy link
Contributor Author

mikicho commented Sep 17, 2024

This is more of a nock-side issue, which I don't think we should tackle on the interceptors side.
The gist is that nock used to be sync (which is incompatible with how Node) and it was safe to call reset in the middle of a request. Currently, I have just dropped "support" for it and I'm waiting to see if anyone complains

@kettanaito
Copy link
Member

I lack enough familiarity with Nock to say this for certain, but applying/removing mocking in a middle of a request sounds like a bad idea. Imo, the request must be handled according to the interception state at the time of being issued.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants