Skip to content

feat: Adds support for inserting middleware for operations - #332

Merged
maxieduncan merged 3 commits into
mainfrom
operation_middlware
Aug 24, 2026
Merged

maxieduncan merged 3 commits into
mainfrom
operation_middlware

Conversation

@maxieduncan

Copy link
Copy Markdown
Contributor

This allows middleware such as circuit breakers to be inserted and executed during commercetools requests.

This allows middleware such as circuit breakers to be inserted and executed during commercetools requests.
@codecov

codecov Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.32%. Comparing base (b8f2977) to head (8c3284e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #332   +/-   ##
=======================================
  Coverage   99.32%   99.32%           
=======================================
  Files          58       58           
  Lines        1475     1481    +6     
  Branches      156      158    +2     
=======================================
+ Hits         1465     1471    +6     
  Misses          2        2           
  Partials        8        8           
Files with missing lines Coverage Δ
src/lib/api/CommercetoolsApi.ts 99.14% <ø> (ø)
src/lib/auth/CommercetoolsAuthApi.ts 97.43% <ø> (ø)
src/lib/request/request-executor.ts 100.00% <100.00%> (ø)
src/lib/types.ts 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds configurable operation middleware around commercetools API request execution.

Changes:

  • Defines middleware types and configuration.
  • Composes middleware with request execution.
  • Passes middleware through API configuration.
  • Adds executor and API integration tests.

CommercetoolsAuthApi does not pass configured middleware to its executor, so middleware is skipped for authentication calls. This must be addressed before approval.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Summary
src/test/request/__tests__/request-executor.test.ts Tests middleware behavior.
src/test/api/CommercetoolsApi.test.ts Tests API middleware integration.
src/lib/types.ts Defines middleware configuration and exposes the unresolved auth propagation issue.
src/lib/request/request-executor.ts Composes middleware around requests.
src/lib/api/types.ts Exposes middleware in API configuration.
src/lib/api/CommercetoolsApi.ts Passes middleware to the API executor.
Suppressed comments (2)

src/lib/api/CommercetoolsApi.ts:331

  • operationMiddlewares is now part of CommercetoolsBaseConfig, so it is accepted by CommercetoolsAuthConfig/CommercetoolsAuthApiConfig, but this only wires it into the API executor. CommercetoolsAuthApi still constructs its executor without operationMiddlewares (src/lib/auth/CommercetoolsAuthApi.ts:46-53), so direct auth calls—and the token request made while preparing an API call—silently bypass the configured middleware. Forward the option in the auth executor as well, or remove it from the shared auth config if authentication is intentionally out of scope.
      operationMiddlewares: config.operationMiddlewares,

src/lib/request/request-executor.ts:54

  • The new documentation says middleware may call next without a modified request, but next is typed as RequestExecutor, whose request argument is required, and this composition forwards the argument directly. A documented next() call therefore fails type-checking and, at runtime, reaches baseExecutor with undefined and throws when it reads requestConfig.headers. Make no-argument continuation default to the current request (including the type) or update the contract to require next(requestConfig).
  const composedExecutor = middlewares.reduceRight<RequestExecutor>((next, middleware) => {
    const executor: RequestExecutor = (requestConfig: CommercetoolsRequest) => middleware(next, requestConfig)
    return executor

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/lib/types.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/lib/api/CommercetoolsApi.ts:331

  • This only installs the middleware on the API executor, but CommercetoolsApi.request() calls getRequestOptions() first and that method may fetch a client grant through this.auth; CommercetoolsAuthApi also creates its own executor without forwarding operationMiddlewares. Consequently auth requests (including the implicit token request before the first API call) bypass a middleware advertised as wrapping a full logical commercetools operation, so a circuit breaker or short-circuit cannot protect those calls. Either apply the same middleware pipeline to the auth executor as well, or narrow the option's documentation and name to explicitly state that it covers API calls only.
      operationMiddlewares: config.operationMiddlewares,

src/lib/api/types.ts:18

  • This middleware is invoked only after request() has awaited getRequestOptions(), which calls auth.getClientGrant() when no grant is cached. Consequently, an open circuit or other short-circuit still makes the token HTTP request on the first API operation, so the middleware does not actually wrap the full logical operation described here and cannot prevent all outbound calls. Either move token acquisition inside the middleware boundary or scope the option/documentation explicitly to the API request.
   * Middleware pipeline that wraps a full logical request operation.
   *
   * Each middleware receives the next executor in the chain and the request
   * config for the current operation. Middleware can:
   * - call `next(requestConfig)` to continue,
   * - short-circuit by returning a value without calling `next`, or
   * - throw to fail the operation.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@maxieduncan
maxieduncan marked this pull request as ready for review August 24, 2026 08:36
@maxieduncan maxieduncan changed the title Adds support for inserting middleware for operations feat: Adds support for inserting middleware for operations Aug 24, 2026
@maxieduncan
maxieduncan merged commit 5b75f82 into main Aug 24, 2026
6 checks passed
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 6.30.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants