feat: add timeout to api client

[JimTacobs]

Description

Added a timeout param that can be passed when creating an API client. This timeout is passed directly into ofetch.create, leveraging its functionality in setting up a default timeout for every request made with the client.

Created three relevant test cases that show that the timeout property does what it's supposed to do.

Created another two tests to show that the timeout can be overwritten using fetchOptions

I have opted against expanding the onResponseError method with error handling specific for timeout errors, as I am confident the already available and in use errorInterceptor will be sufficient. If users do want more, they can simply hook into onResponseError and add their own custom implementation.

Added documentation to show:

• How to add a default timeout when creating your client

As I wasn't sure whether you'd also want it on the admin API client, I've omitted it here.

Type of change

New feature (non-breaking change which adds functionality)

ToDo's

None

Additional context

None

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
shopware-frontends-docs	✅ Ready (Inspect)	Visit Preview	Dec 4, 2024 10:58am

[patzick]

Thanks for your contribution @JimTacobs!
I'd like to hear more about your story behind the global timeout, could you share the usecase here?

Currently you can already set timeout for specific requests like

 const request = client.invoke("getOrderList get /order", {
      fetchOptions: {
        timeout: 200
      },
    });

Not saying we shouldn't expand it to global case as well, but the usecase would be very useful here :)

[JimTacobs]

@patzick

Certainly! I can see a few advantages to having the timeout set as a default on the HTTP client itself.

Consistency During Migration:
Having a global timeout allows for a smoother transition from the old client to your new API client. It eliminates the need to add the timeout to every request manually or refactor our code to wrap each request. This makes the migration process less error-prone and faster to implement.

Centralized Configuration:
Setting the timeout in one central place (the HTTP client) simplifies maintenance. When we need to change the timeout value, we know exactly where to look and only need to update it in one place. This also ensures that every request has a timeout, preventing potential issues where a timeout might accidentally be omitted. For specific endpoints that require a different timeout, it’s clear that their configuration is an override of the global default.

Alignment with ofetch Standards:
Since ofetch supports global timeouts, it felt natural to look for this feature in the API client. Maintaining this consistency not only aligns with ofetch’s behavior but also helps meet the expectations of developers familiar with it. Personally, not being able to set a global timeout initially deterred me from fully adopting the API client a couple of months ago, as it added extra overhead to our implementation.

I hope this makes sense and aligns with the vision for the client. I’d be happy to discuss or refine this further based on your feedback!

[patzick]

I like this reasoning 🙌, let's make a tiny bit more out of it

• global fetch options type picking from the current "retry", "retryDelay", "retryStatusCodes", "timeout"
• let's make the API the same as in single method, so optional fetchOptions param with these values - this will make potential extending easier and it's clearer which params are 1:1 inherited from the ofetch
• both API clients should support this

what would you say on that?

[JimTacobs]

@patzick Yes, that would all make perfect sense to also include in this PR! I'll find some time this week to add these points!

[JimTacobs]

@patzick All your requests have been added to the PR.

[pkg-pr-new]

Open in Stackblitz

@shopware/api-client

npm i https://pkg.pr.new/shopware/frontends/@shopware/api-client@1468

@shopware/api-gen

npm i https://pkg.pr.new/shopware/frontends/@shopware/api-gen@1468

@shopware-pwa/composables-next

npm i https://pkg.pr.new/shopware/frontends/@shopware-pwa/composables-next@1468

@shopware-pwa/helpers-next

npm i https://pkg.pr.new/shopware/frontends/@shopware-pwa/helpers-next@1468

@shopware-pwa/nuxt3-module

npm i https://pkg.pr.new/shopware/frontends/@shopware-pwa/nuxt3-module@1468

@shopware-pwa/cms-base

npm i https://pkg.pr.new/shopware/frontends/@shopware-pwa/cms-base@1468

commit: ecc1b90

[codspeed-hq]

CodSpeed Performance Report

Merging #1468 will not alter performance

_{Comparing JimTacobs:feat/add-timeout-to-api-client (ecc1b90) with main (0d426c2)}

Summary

✅ 9 untouched benchmarks

[patzick]

Great stuff! Thank you for your contribution 🙌