feat: Add Middleware for HTTP-request compression by davidkna-sap · Pull Request #6291 · SAP/cloud-sdk-js

davidkna-sap · 2026-01-30T13:13:51Z

Required for SAP/ai-sdk-js-backlog#470

marikaner

I like this😊

packages/http-client/src/compress-request-middleware.ts

marikaner · 2026-02-11T09:04:26Z

packages/http-client/src/compress-request-middleware.ts

+   * - If `true` is provided, the payload will always be compressed.
+   * - If `false` is provided, the payload will never be compressed.


[pp] I find it a little odd, that this is a combination of boolean and strings. Consider "always" and "never" as values. That is more explicit. If that is common practice, it's fine.

I've renamed it to always and never. In addition, I've also renamed passthrough to header-only WDYT?

I think it's good.

ZhongpinWang · 2026-02-11T11:50:15Z

packages/http-client/src/compress-request-middleware.ts

+  if (mode === 'never') {
+    return false;
+  }
+  if (mode === 'always') {
+    return true;
+  }


[pp] Minor preference. Since we are using 'always' and 'never', maybe also return here string literal instead of true false. Just a pp.

[pp] Sorry to reopen this can... I disagree. This only makes things more complicated... This function should return a boolean, but only do what its name suggests: check if the payload should be compressed. Later, when it is used, we can then do this:

if (needsCompression || options.mode === 'header-only') { // add header } if (needsCompression) { // compressPayload } // call middleware chain

[pp] Also, for better readability and brevity I would like to suggest the following flow for this function:

if (mode === "auto") { // do the compression decision return decision; // true or false } return mode === "always";

ZhongpinWang · 2026-02-11T11:52:18Z

packages/http-client/src/compress-request-middleware.ts

+    throw new ErrorWithCause(
+      "Could not determine payload size for 'auto' compression decision.",
+      e
+    );


[pp/req] Should we throw or rather do a soft warning log and fallback to not compressing option? I find it a bit annoying if people use auto but the whole app crashes without fallback because of our implementation failed.
[q] When will Buffer.byteLength() throw?

I guess other types does not make sense to be sent in the context of the RPT service?

But looking at the function name checkIfNeedsCompression() it is supposed to just check if needed and this is Cloud SDK which is supposed to support all request data types. I still feel bad if we throw something in a check function. Not a clean design I would say. Also we are technically allowing user sending for example number to remote if using never. Just want to make the responsibility of this function clear.

I would fallback to return false in case we cannot compress.

@ZhongpinWang

I guess other types does not make sense to be sent in the context of the RPT service?

For the parquet-endpoint FormData will be used too, but parquet is a binary format which can have internal compression. Compression with FormData might be per-item anyway.

I would fallback to return false in case we cannot compress.

The compress functions have the same <string> | <Buffer> | <TypedArray> | <DataView> | <ArrayBuffer> | <SharedArrayBuffer> types, so I moved the payload size check to also run in always-mode to catch incompatible types.

In think throwing for incompatible compression algorithms is fine.

packages/http-client/src/compress-request-middleware.ts

marikaner · 2026-02-12T07:13:46Z

packages/http-client/src/compress-request-middleware.ts

+   * - If `true` is provided, the payload will always be compressed.
+   * - If `false` is provided, the payload will never be compressed.


I think it's good.

marikaner · 2026-02-12T07:21:48Z

packages/http-client/src/compress-request-middleware.ts

+    const currentValue = pickValueIgnoreCase(
+      requestConfig.headers,
+      'content-encoding'
+    );
+    const algorithmValue = getContentEncodingValue(algorithm);
+    const targetValue = currentValue
+      ? `${currentValue}, ${algorithmValue}`
+      : algorithmValue;
+    requestConfig.headers = mergeIgnoreCase(requestConfig.headers, {
+      'content-encoding': targetValue
+    });


[pp] I would again extract this to a separate function (e.g. addContentEncodingHeader()). That way the function name would replace the comment. I also think that the name currentValue is not very speaking in the context of this function, but good enough in an extracted function, that deals with the headers only.

I tried to change this, but did not like how it ended up looking, so I opted to rename the variables here.

Co-authored-by: Marika Marszalkowski <marika.marszalkowski@sap.com>

Rename the middleware export and all imports from compressRequest() to compress() across the HTTP client package and tests. Update the changeset note to reference compress(). Keep the middleware API and options unchanged; the generic type parameter and options remain the same. Adjust unit tests to import and call compress() so all compression modes, algorithms (gzip, brotli, deflate, zstd), and payload types continue to be validated. Clarify naming to reduce confusion between request/response semantics while preserving existing runtime behavior.

marikaner

Only minor stuff. LGTM 😊

packages/http-client/src/compress-request-middleware.ts

marikaner · 2026-02-26T07:47:31Z

packages/http-client/src/compress-request-middleware.ts

+  if (mode === 'never') {
+    return false;
+  }
+  if (mode === 'always') {
+    return true;
+  }


[pp] Sorry to reopen this can... I disagree. This only makes things more complicated... This function should return a boolean, but only do what its name suggests: check if the payload should be compressed. Later, when it is used, we can then do this:

if (needsCompression || options.mode === 'header-only') { // add header } if (needsCompression) { // compressPayload } // call middleware chain

marikaner · 2026-02-26T07:58:10Z

packages/http-client/src/compress-request-middleware.ts

+  if (mode === 'never') {
+    return false;
+  }
+  if (mode === 'always') {
+    return true;
+  }


[pp] Also, for better readability and brevity I would like to suggest the following flow for this function:

if (mode === "auto") { // do the compression decision return decision; // true or false } return mode === "always";

packages/http-client/src/compress-request-middleware.ts

feat: Add Middleware for HTTP-request compression

828bbee

davidkna-sap force-pushed the davidkna-sap_feat-compress-middleware branch from 47dc29e to 828bbee Compare January 30, 2026 13:15

davidkna-sap marked this pull request as ready for review January 30, 2026 14:27

davidkna-sap requested review from KavithaSiva, emincihangeri and marikaner January 30, 2026 14:27

davidkna-sap mentioned this pull request Jan 30, 2026

feat: Support compressed requests in the RPT client SAP/ai-sdk-js#1488

Open

refactor: Simplify middleware implementation

bbc9f9e

davidkna-sap mentioned this pull request Feb 2, 2026

chore: [JS] Document Request Compression Middleware SAP/cloud-sdk#2369

Draft

KavithaSiva and others added 2 commits February 3, 2026 11:11

Merge branch 'main' into davidkna-sap_feat-compress-middleware

b9f30a8

simplify per-algorithm options handling

9d286c1

davidkna-sap added blocked Issues that require async communication or are blocked by external factors and removed blocked Issues that require async communication or are blocked by external factors labels Feb 6, 2026

marikaner approved these changes Feb 11, 2026

View reviewed changes

avoid boolean for mode, rename passthrough to header-only

efc6891

davidkna-sap requested a review from marikaner February 11, 2026 09:47

ZhongpinWang reviewed Feb 11, 2026

View reviewed changes

davidkna-sap and others added 2 commits February 11, 2026 14:55

address review comments

c8616ce

Changes from lint:fix

9d80c18

marikaner approved these changes Feb 12, 2026

View reviewed changes

davidkna-sap and others added 7 commits February 12, 2026 09:00

Update packages/http-client/src/compress-request-middleware.ts

d11cfb4

Co-authored-by: Marika Marszalkowski <marika.marszalkowski@sap.com>

Changes from lint:fix

b582635

rename variables for clarity

7c682a1

always check payload size to avoid issues later during compression

5b82b7a

Merge branch 'main' into davidkna-sap_feat-compress-middleware

a2b8b8d

Changes from lint:fix

b5ad0c3

marikaner approved these changes Feb 26, 2026

View reviewed changes

address review feedback

b86d84a

		* - If `true` is provided, the payload will always be compressed.
		* - If `false` is provided, the payload will never be compressed.

Conversation

davidkna-sap commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marikaner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZhongpinWang Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZhongpinWang Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidkna-sap Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marikaner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

davidkna-sap commented Jan 30, 2026 •

edited

Loading

ZhongpinWang Feb 11, 2026 •

edited

Loading

ZhongpinWang Feb 16, 2026 •

edited

Loading

davidkna-sap Feb 16, 2026 •

edited

Loading