Repository navigation
perf!: serialize requests once in batchAddRequests - #1051
Conversation
|
See more at https://github.com/apify/apify-client-js/actions/runs/34862046980#summary-104036487629 |
…-single-serialization
…-single-serialization # Conflicts: # docs/04_upgrading/upgrading_v3.md # docs/public-api/apify-client.api.md # src/resource_clients/request_queue.ts # test/http_client.test.ts
…-single-serialization # Conflicts: # src/resource_clients/request_queue.ts
|
|
||
| A string body sent with an explicit `application/json` content type now goes out verbatim. Axios used to parse it to check that it was valid JSON, and wrapped it in a JSON string literal when it wasn't. The client skips that step so that `batchAddRequests()` can send a body it has already serialized. | ||
|
|
||
| <ApiLink to="class/KeyValueStoreClient#setRecord">`setRecord()`</ApiLink> is where you'd notice. Storing a non-JSON string under `contentType: 'application/json'` used to save `"my value"`, quotes included, and now saves `my value`, which <ApiLink to="class/KeyValueStoreClient#getRecord">`getRecord()`</ApiLink> can't parse back: |
There was a problem hiding this comment.
which
getRecord()can't parse back
This is kinda annoying, but understandable.
I'm not sure whether we should automatically parse the KVS contents based on the content-type in the client, but that ship has sailed, I suppose.
There was a problem hiding this comment.
Yeah, I'm merging this. However, feel free to start a follow-up Slack discussion about it. I'd probably lean toward dropping this automatic parsing of KVS records based on their content type, but I think it deserves a bit more investigation and discussion first with a broader audience (also #product-dev-tools channel). I opened a follow-up issue for it: #1067, and we should discuss it before the v3 release.
Adds a regression test for #277: `start()` with a `Buffer` input and `contentType: 'application/json'` sends the raw bytes, and the server gets the decoded JSON back. The bug itself is already gone on `v3`. `serializeBody()` from #1058 passes binary and string bodies through before it reaches `JSON.stringify`, and axios no longer re-serializes strings since #1051 set `transformRequest: []`. With the Buffer branch disabled, the new test fails with the `{ type: 'Buffer', data: [...] }` payload from the issue. Closes #277 *✍️ Drafted by Claude Code*
### Description - Mirroring apify/apify-client-python#953. - `batchAddRequests` stringified every request twice: once in `sliceArrayByByteLength` to measure the batch, and again in the `serializeRequest` interceptor when sending. Axios then ran its default `transformRequest` on top, because `transformRequest: undefined` in the instance config falls back to the defaults, and that default validates a JSON string body by parsing it in full. Each batch body was stringified twice and parsed once more. - Each request is now serialized once up front. The byte lengths decide the batch boundaries (commas and brackets counted, which the old measurement skipped) and the same strings are joined into the batch body, sent with an explicit `content-type: application/json`. The interceptor passes a string body with an explicit content type through untouched, and the axios instance sets `transformRequest` and `transformResponse` to `[]`. That also removes the validation re-parse for every JSON body the client sends. ### Issue - Closes #972 ### Breaking changes - A string body declared as JSON but not valid JSON (for example `setRecord` with `contentType: 'application/json'` and a non-JSON string) is sent as it is. Axios used to double-encode it into a JSON string literal. - A request too large for the payload limit is rejected before any batch is sent. It used to be detected only when its batch came up, with earlier batches already in flight. - The protected `_batchAddRequests` and `_batchAddRequestsWithRetries` take the serialized entries, and `_batchAddRequests` no longer re-validates a batch the public method already validated. The API report is updated. *✍️ Drafted by Claude Code*
Adds a regression test for #277: `start()` with a `Buffer` input and `contentType: 'application/json'` sends the raw bytes, and the server gets the decoded JSON back. The bug itself is already gone on `v3`. `serializeBody()` from #1058 passes binary and string bodies through before it reaches `JSON.stringify`, and axios no longer re-serializes strings since #1051 set `transformRequest: []`. With the Buffer branch disabled, the new test fails with the `{ type: 'Buffer', data: [...] }` payload from the issue. Closes #277 *✍️ Drafted by Claude Code*
Description
batchAddRequestsstringified every request twice: once insliceArrayByByteLengthto measure the batch, and again in theserializeRequestinterceptor when sending. Axios then ran its defaulttransformRequeston top, becausetransformRequest: undefinedin the instance config falls back to the defaults, and that default validates a JSON string body by parsing it in full. Each batch body was stringified twice and parsed once more.content-type: application/json. The interceptor passes a string body with an explicit content type through untouched, and the axios instance setstransformRequestandtransformResponseto[]. That also removes the validation re-parse for every JSON body the client sends.Issue
batchAddRequests#972Breaking changes
setRecordwithcontentType: 'application/json'and a non-JSON string) is sent as it is. Axios used to double-encode it into a JSON string literal._batchAddRequestsand_batchAddRequestsWithRetriestake the serialized entries, and_batchAddRequestsno longer re-validates a batch the public method already validated. The API report is updated.✍️ Drafted by Claude Code