Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support no-store vs no-cache on fetchClient to work w Cloudflare workers outside typical node workflow #246

Closed
dorianjp opened this issue Dec 13, 2024 · 1 comment · Fixed by #249

Comments

@dorianjp
Copy link

Here's an updated version of the PR description that incorporates the Cloudflare context and your use case:


PR: Patch for @axiomhq/[email protected] to Resolve Cache Issue with Cloudflare Integration

Hi there! 👋

First, thank you for your work on this library and for making logging ingestion so seamless! 😊

While using @axiomhq/[email protected] in my project, I encountered an issue related to the cache setting when integrating Axiom with a Cloudflare environment. Specifically, Cloudflare does not support cache: 'no-cache', which caused unexpected behavior.

Additionally, while Axiom provides a dedicated Cloudflare Worker module, it isn't suitable for all Cloudflare Worker frameworks. In my case, I’m integrating Axiom ingestion with SvelteKit’s hooks for a server-side logging setup, and the Worker module doesn’t align well with this framework’s architecture.

Problem Description

  • Environment: I’m using SvelteKit deployed on Cloudflare Pages, which relies on Cloudflare Workers for server-side functionality.
  • Issue: The current cache: 'no-cache' setting in the FetchClient conflicts with Cloudflare’s implementation, which doesn’t fully respect the no-cache directive. This results in inconsistent behavior where caching occurs unintentionally.
  • Impact: Requests that should bypass intermediate caches sometimes fail to do so, leading to potential data discrepancies or stale responses.

Solution

To resolve this, I’ve patched the library to replace cache: 'no-cache' with cache: 'no-store'. This ensures that no intermediate caching occurs, as no-store is fully supported by Cloudflare and more reliably enforces uncached requests.

Files Updated

The patch modifies the following files to update the cache setting:

  1. dist/cjs/fetchClient.cjs
  2. dist/esm/fetchClient.js
  3. src/fetchClient.ts

Here’s the relevant diff:

diff --git a/node_modules/@axiomhq/js/dist/cjs/fetchClient.cjs b/node_modules/@axiomhq/js/dist/cjs/fetchClient.cjs
index d17e816..cd7c9ab 100644
--- a/node_modules/@axiomhq/js/dist/cjs/fetchClient.cjs
+++ b/node_modules/@axiomhq/js/dist/cjs/fetchClient.cjs
@@ -25,7 +25,7 @@ class FetchClient {
             method,
             body: init.body ? init.body : undefined,
             signal: AbortSignal.timeout(timeout),
-            cache: 'no-cache',
+            cache: 'no-store',
         });
         if (resp.status === 204) {
             return resp;
diff --git a/node_modules/@axiomhq/js/dist/esm/fetchClient.js b/node_modules/@axiomhq/js/dist/esm/fetchClient.js
index 7a60e2d..186356b 100644
--- a/node_modules/@axiomhq/js/dist/esm/fetchClient.js
+++ b/node_modules/@axiomhq/js/dist/esm/fetchClient.js
@@ -23,7 +23,7 @@ class FetchClient {
             method,
             body: init.body ? init.body : undefined,
             signal: AbortSignal.timeout(timeout),
-            cache: 'no-cache',
+            cache: 'no-store',
         });
         if (resp.status === 204) {
             return resp;
diff --git a/node_modules/@axiomhq/js/src/fetchClient.ts b/node_modules/@axiomhq/js/src/fetchClient.ts
index 8f3d0b3..b872f18 100644
--- a/node_modules/@axiomhq/js/src/fetchClient.ts
+++ b/node_modules/@axiomhq/js/src/fetchClient.ts
@@ -2,7 +2,7 @@ import fetchRetry from 'fetch-retry';
 import { parseLimitFromResponse, Limit, LimitType } from './limit.js';
 
 export class FetchClient {
-  constructor(public config: { headers: HeadersInit; baseUrl: string; timeout: number }) {}
+  constructor(public config: { headers: HeadersInit; baseUrl: string; timeout: number }) { }
 
   async doReq<T>(
     endpoint: string,
@@ -29,7 +29,7 @@ export class FetchClient {
       method,
       body: init.body ? init.body : undefined,
       signal: AbortSignal.timeout(timeout),
-      cache: 'no-cache',
+      cache: 'no-store',
     });
 
     if (resp.status === 204) {

Request

If this change aligns with the goals of the library, I kindly request:

  1. Review and consider integrating this patch: It resolves a common compatibility issue for Cloudflare-based users.
  2. Explore additional support for Cloudflare frameworks: A solution bridging the gap between the general @axiomhq/js package and the specialized Worker module could benefit users working with frameworks like SvelteKit or other server-side tools.

Thank you for your time and consideration! I’m happy to provide additional context or assist further if needed. 😊

This issue body was partially generated by patch-package.

dasfmi added a commit that referenced this issue Dec 16, 2024
This fix closes #246; Cloudflare doesn't support no-cache value
so this change sets cache to 'no-store' instead.
@dasfmi
Copy link
Collaborator

dasfmi commented Dec 16, 2024

hi @dorianjp, thanks for your contribution! I have created a PR with the change, glad to help. Once its approved by the team, a new release will be pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants