feat(nuxt-module): implement apiClientConfig.timeout

[Maciek Kucmus (mkucmus)]

Description

closes #2690

apiClientConfig.timeout was declared on ShopwareNuxtOptions but read by nothing. Anyone who set it got no timeout and no warning.

timeout now sits on ApiClientConfig next to headers, the surface plugin.ts already reads, and is forwarded to createAPIClient as fetchOptions.timeout. Resolution order, highest first:

runtimeConfig.apiClientConfig.timeout          (SSR only)
runtimeConfig.public.apiClientConfig.timeout
shopware.apiClientConfig.timeout               (SSR only, deprecated)
public shopware.apiClientConfig.timeout        (deprecated)

Each level is validated on its own before falling through, so an invalid high-priority value never blocks a valid lower one. A finite positive number is valid, and a numeric string is coerced. 0, negatives, Infinity, NaN, non-numeric strings, booleans and objects are ignored and logged once as a warning naming the config path the value came from.

apiClientConfig under the shopware module options is kept, marked @deprecated, and read last. Removing it would break type-checking for anyone who set it, and this is not a major release. Move to runtimeConfig.apiClientConfig; the fallback goes away in the next major.

Shipped unset. No default, and no template sets it. Retry is not exposed: fetchOptions is built literally as { timeout }, so retry, retryDelay and retryStatusCodes stay unreachable.

Type of change

New feature (non-breaking change which adds functionality)

ToDo's

• Changeset file provided read more
• Documentation added/updated
• Unit-Tests added/updated
• Related Issue updated

Additional context

The changeset is minor, not the patch the issue suggested. The issue's note assumed the change only drops a dead declaration, but it also adds an option that works at a path that did not exist before. New options ship as minor here: cacheableReads in 1.5.0, browserLocale in 1.4.0.

plugin.ts had no test at all. It has one now, which needed a vitest define shim so import.meta.server can be flipped per test. Behavioural tests were added in packages/api-client for the timeout error shape and for request-level signal.

Worth knowing before enabling it: a timed-out GET consumes its automatic retry without re-reaching the server, and the resulting error carries no HTTP status, so instanceof ApiClientError does not match it. Use isTimeoutError() from @shopware/api-client to detect one. Both are covered by tests.

#2702 has merged. This branch now uses isTimeoutError() in both checkout pages, drops the two api-client tests it superseded, and documents that a per-call signal combines with the timeout instead of replacing it.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
frontends-starter-template-extended	Ready	Preview	Sep 9, 2026 7:39am UTC
frontends-vue-starter-template	Ready	Preview	Sep 9, 2026 7:39am UTC

[Copilot]

🟡 Changes recommended

Runtime coercion accepts invalid boolean values, and the changeset level conflicts with the linked issue.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Implements configurable Store API request timeouts in the Nuxt module.

Changes:

• Forwards validated runtime timeout configuration to the API client.
• Adds plugin tests and user documentation.
• Removes the ineffective module option and adds release metadata.

File summaries

File	Description
packages/nuxt-module/plugin.ts	Applies runtime timeout configuration.
packages/nuxt-module/plugin.test.ts	Tests timeout forwarding and precedence.
packages/nuxt-module/vitest.config.ts	Enables server/client branch testing.
packages/nuxt-module/types/runtime-config.d.ts	Declares the runtime timeout option.
packages/nuxt-module/src/index.ts	Removes the ineffective module option.
packages/nuxt-module/README.md	Documents timeout behavior and configuration.
apps/e2e-tests/diagnostics/network-failures.jsonl	Records observed network failures.
.changeset/olive-pandas-wait.md	Describes the package release.

Review details

• Files reviewed: 7/8 changed files
• Comments generated: 3
• Review effort level: Balanced

---

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

[Copilot]

🟡 Changes recommended

The documentation incorrectly describes signal behavior, and an unrelated transient diagnostics artifact should be removed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

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

apps/e2e-tests/diagnostics/network-failures.jsonl:13

• This is transient output from one E2E run, and no repository code generates or consumes this diagnostics file. Keeping environment-specific failures in source will become stale and is unrelated to the timeout API; remove the artifact (and ignore its output path if a local tool recreates it).

• Files reviewed: 7/8 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The deprecated module-option declaration conflicts with the implementation path stated in the PR description and linked issue.

Review details

Suppressed comments (1)

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

packages/nuxt-module/src/index.ts:153

• The PR description and linked issue say this non-functional module-option surface is removed (or alternatively made functional), but this keeps it in the public type while the plugin still never reads it. Deprecating it documents the mismatch but leaves the third outcome that #2690 explicitly excludes; either remove the property to match the stated implementation path, or wire this scope through and update the PR/docs accordingly.

• Files reviewed: 8/9 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Maciek Kucmus (mkucmus)]

Maciej D (@mdanilowicz) following your recommendation, the api-client part goes first.

isTimeoutError() and combining a caller signal with the configured timeout are ready on feat/api-client-timeout-signal and will open as a separate PR. nuxt-module and the templates consume api-client, so that order keeps each change in one package.

Merge order:

1. The api-client PR merges.
2. This PR merges main, drops the test that asserts the old signal behaviour, fixes the two README bullets, and can use isTimeoutError() in the checkout branches.
3. Re-review here.

Until then this PR stays as is, so its CI does not go red before the api-client change is in. I will link the PR here once it is open.

[Patryk Tomczyk (patzick)]

🔍 Code Review

The plumbing is right: toTimeout → fetchOptions.timeout, four-level precedence, tests that actually fail when the fallback order is swapped, and both checkout pages no longer swallow a non-ApiClientError. CI is fully green (11/11).

Four things I'd like resolved before merge.

💥 Blockers / Majors

1. An invalid timeout is still silently ignored — plugin.ts:58. The PR's own premise is "anyone who set it got no timeout and no warning". toTimeout reproduces exactly that for "5000", 0, -1, NaN, Infinity: rejected, no diagnostic anywhere. Warn when the key is present but rejected.

2. The type narrowing is a breaking change under a minor — src/index.ts:163. timeout?: number | string → timeout?: number. "5000" was type-legal before; those users get a type error on a same-major bump, and per (1) no runtime signal either.

3. Checkout now shows a generic error where the order may already exist — vue-demo-store/.../checkout/index.vue:246, vue-starter-template/.../checkout/index.vue:62. Visible beats silent, so this is the right direction — but "something went wrong" plus a re-armed button is an invitation to press it again, and a timeout is precisely the case where the server may have committed. A timeout-specific message would close the gap.

4. Merge-order coupling isn't encoded anywhere — createApiClient.test.ts:737, README.md:143. Both assert the ofetch 1.5.1 signal-replaces-timeout behaviour that #2702 deliberately removes. Whichever lands first, the other is wrong. Your plan in the thread is fine; I'd just rather see #2702 merged and this branch updated than the ordering living only in a comment.

🧩 Minors / Nits

• plugin.ts:122-123 — the two deprecated tiers can never differ in practice: src/index.ts:29-35 defus module options into both runtimeConfig.shopware and runtimeConfig.public.shopware, so shopware.apiClientConfig.timeout always lands in both. The precedence test builds them independently, which no real nuxt.config can produce.
• plugin.ts:109 — the local ApiClientConfig (timeout?: unknown) duplicates the augmented interface in types/runtime-config.d.ts (timeout?: number). Two declarations that can drift.
• src/index.ts:50 — the warning fires on the mere presence of the object, so apiClientConfig: {} warns about moving a timeout that was never set.
• apps/e2e-tests/.gitignore:4 — unrelated to the timeout API.

✅ Verified

• errors.message-default exists in all three locales of both templates.
• useApiErrorsResolver().handleApiError has the non-ApiClientError branch this relies on.
• The vitest define shim is behaviour-neutral for the existing suite — import.meta.server was already undefined under plain vitest.

Verdict: changes requested — (1) and (4) are the ones that matter; (2) and (3) are quick.

[cursor]

Stale comment

Security review

No medium, high, or critical vulnerabilities in this update (211f4bb).

apiClientConfig.timeout is operator-controlled Nuxt runtime/module config. toTimeout accepts only a finite positive number (numeric strings from env are coerced; 0, negatives, NaN, Infinity, booleans, and objects are dropped). The value is forwarded as fetchOptions: { timeout } only — retry knobs stay unreachable. Private config remains SSR-only; private headers are still not read in the browser.

Checkout timeout handling uses static i18n strings (errors.order-timeout). That is availability/UX (including a possible already-placed order), not an attacker-controlled path.

No prior automation security-review threads to re-validate. No new dependencies.

  

_{Sent by Cursor Automation: Review pull requests for exploitable security issues and flag only validated findings before merge}

[cursor]

Stale comment

Security review

No medium, high, or critical vulnerabilities in this update (f1e1829).

apiClientConfig.timeout is operator-controlled Nuxt runtime/module config. toTimeout accepts only a finite positive number (numeric strings from env are coerced; 0, negatives, NaN, Infinity, booleans, and objects are dropped and warned). The value is forwarded as fetchOptions: { timeout } only — retry knobs stay unreachable. Private apiClientConfig remains SSR-only; private headers are still not read in the browser.

Checkout timeout handling uses static i18n strings (errors.order-timeout). That is availability/UX (including a possible already-placed order), not an attacker-controlled path.

Delta since the last review is a merge of main (release, CI, dependency audit). No new dependencies or security-sensitive behavior in the timeout plumbing.

No prior automation security-review finding threads to re-validate.

  

_{Sent by Cursor Automation: Review pull requests for exploitable security issues and flag only validated findings before merge}

[Patryk Tomczyk (patzick)]

✅ Recheck on f1e1829 — all eight findings addressed

CI green on the current head (10 check-runs + 3 statuses, all success). Superseding my earlier CHANGES_REQUESTED.

#	Finding	Resolution
1	Rejected timeout silently dropped	toTimeout now takes a source, coerces numeric strings, and warns once per distinct message naming the config path — plugin.ts:58-80
2	number | string → number type break	Reverted to number | string on both tiers via the shared ApiClientRuntimeConfig — src/index.ts:144-149
3	Generic checkout error after a possibly-committed order	Dedicated errors.order-timeout ("your order may still have been placed…") in both templates, all three locales
4	Test/README coupled to unmerged #2702	Test renamed ofetch 1.5.1 only: … (deleted by #2702), README bullet now scopes the claim and links the PR
5	Dead precedence tier	Comment added; both deprecated tiers report as shopware.apiClientConfig, the name nuxt.config actually uses
6	Duplicated ApiClientConfig declaration	Collapsed — runtime-config.d.ts is now interface ApiClientConfig extends ApiClientRuntimeConfig {}
7	Warning fired on an empty object	Guarded on ?.timeout !== undefined — src/index.ts:50-51
8	Out-of-scope .gitignore entry	Reverted cleanly, byte-for-byte

What I re-verified

• undefined/null short-circuit before the warn path, so "unset" stays silent — tested explicitly.
• The dedupe Set is module-scoped, and vi.resetModules() in beforeEach gives each test a fresh one, so "warns once for a repeated bad value" is testing the real thing rather than leaked state.
• The precedence fixtures were updated for string coercion — the old "2000" invalid-value cases became "2000ms", which is still rejected. Easy thing to miss.
• errors.order-timeout is present in de-DE, en-GB and pl-PL for both templates.

Non-blocking

The isTimeoutError stopgap is duplicated in both checkout pages. Both carry the // Replace with isTimeoutError() from @shopware/api-client once #2702 lands. comment, so it's traceable — just two places to remember.

Accepting string coercion on the new runtimeConfig.apiClientConfig path (not just the deprecated one) is looser than I'd have picked for a fresh option, since Nuxt's destr already turns NUXT_* env values into numbers. It's documented in both the README and the changeset, so it's a deliberate call and fine by me.

Maciej D (@mdanilowicz)'s earlier request for changes is still open and is theirs to clear — the two api-client items from their review are tracked in #2702.

[cursor]

Security review

No medium, high, or critical vulnerabilities in this update (fc259e18).

apiClientConfig.timeout remains operator-controlled Nuxt runtime/module config. toTimeout accepts only a finite positive number (numeric strings from env are coerced; 0, negatives, NaN, Infinity, booleans, and objects are dropped and warned). The value is forwarded as fetchOptions: { timeout } only — retry knobs stay unreachable. Private apiClientConfig remains SSR-only; private headers are still not read in the browser.

Checkout now calls isTimeoutError() from @shopware/api-client and shows the static errors.order-timeout i18n string. That is availability/UX (including a possible already-placed order), not an attacker-controlled path.

Delta since the last review is a merge of main (#2702 timeout-signal + #2708 generated-types). This branch now consumes isTimeoutError() and documents combined signal+timeout behavior. No new dependencies or attacker-reachable sinks.

No prior automation security-review finding threads to re-validate.

  

_{Sent by Cursor Automation: Review pull requests for exploitable security issues and flag only validated findings before merge}