Fixes Unleash client stops fetching #615 (#616)

* Fixes Unleash client stops fetching #615 * Update repository test for 404 case
Unleash · May 22, 2024 · a8cbb48 · a8cbb48
1 parent 2d561a7
commit a8cbb48
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 62 deletions.
diff --git a/src/metrics.ts b/src/metrics.ts
@@ -225,9 +225,10 @@ export default class Metrics extends EventEmitter {
         httpOptions: this.httpOptions,
       });
       if (!res.ok) {
-        if (res.status === 404 || res.status === 403 || res.status == 401) {
+        if (res.status === 403 || res.status == 401) {
           this.configurationError(url, res.status);
         } else if (
+          res.status === 404 ||
           res.status === 429 ||
           res.status === 500 ||
           res.status === 502 ||

diff --git a/src/repository/index.ts b/src/repository/index.ts
@@ -270,15 +270,7 @@ Message: ${err.message}`,
   // and returns 0 as the next fetch interval (stop polling)
   private configurationError(url: string, statusCode: number): number {
     this.failures += 1;
-    if (statusCode === 404) {
-      this.emit(
-        UnleashEvents.Error,
-        new Error(
-          // eslint-disable-next-line max-len
-          `${url} responded NOT_FOUND (404) which means your API url most likely needs correction. Stopping refresh of toggles`,
-        ),
-      );
-    } else if (statusCode === 401 || statusCode === 403) {
+    if (statusCode === 401 || statusCode === 403) {
       this.emit(
         UnleashEvents.Error,
         new Error(
@@ -294,7 +286,7 @@ Message: ${err.message}`,
   // and return the new interval.
   private recoverableError(url: string, statusCode: number): number {
     let nextFetch = this.backoff();
-    if (statusCode === 429) {
+    if (statusCode === 404 || statusCode === 429) {
       this.emit(
         UnleashEvents.Warn,
         // eslint-disable-next-line max-len
@@ -312,9 +304,10 @@ Message: ${err.message}`,
   }
 
   private handleErrorCases(url: string, statusCode: number): number {
-    if (statusCode === 401 || statusCode === 403 || statusCode === 404) {
+    if (statusCode === 401 || statusCode === 403) {
       return this.configurationError(url, statusCode);
     } else if (
+      statusCode === 404 ||
       statusCode === 429 ||
       statusCode === 500 ||
       statusCode === 502 ||

diff --git a/src/test/metrics.test.ts b/src/test/metrics.test.ts
@@ -252,29 +252,28 @@ test('registerInstance should warn when non 200 statusCode', async (t) => {
   metrics.start();
 });
 
-test('sendMetrics should stop/disable metrics if endpoint returns 404', (t) =>
-  new Promise((resolve) => {
-    const url = getUrl();
-    const metEP = nockMetrics(url, 404);
-    // @ts-expect-error
-    const metrics = new Metrics({
-      metricsInterval: 50,
-      url,
-    });
-
-    metrics.on('warn', () => {
-      metrics.stop();
-      t.true(metEP.isDone());
-      // @ts-expect-error
-      t.true(metrics.disabled);
-      resolve();
-    });
-    metrics.start();
-
-    metrics.count('x-y-z', true);
-
-    metrics.sendMetrics();
-  }));
+test('sendMetrics should backoff on 404', async (t) => {
+  const url = getUrl();
+  nockMetrics(url, 404).persist();
+  const metrics = new Metrics({
+    appName: '404-tester',
+    instanceId: '404-instance',
+    metricsInterval: 10,
+    strategies: [],
+    url,
+  });
+  metrics.count('x-y-z', true);
+  await metrics.sendMetrics();
+  // @ts-expect-error actually a private field, but we access it for tests
+  t.false(metrics.disabled);
+  t.is(metrics.getFailures(), 1);
+  t.is(metrics.getInterval(), 20);
+  await metrics.sendMetrics();
+  // @ts-expect-error actually a private field, but we access it for tests
+  t.false(metrics.disabled);
+  t.is(metrics.getFailures(), 2);
+  t.is(metrics.getInterval(), 30);
+});
 
 test('sendMetrics should emit warn on non 200 statusCode', (t) =>
   new Promise((resolve) => {

diff --git a/src/test/repository.test.ts b/src/test/repository.test.ts
@@ -412,33 +412,6 @@ test.skip('should handle 504 request error and emit warn event', (t) =>
     });
     repo.start();
   }));
-test('should handle 404 request error and emit error event', (t) =>
-  new Promise<void>((resolve) => {
-    const url = 'http://unleash-test-5.app';
-    nock(url).persist().get('/client/features').reply(404, 'asd');
-
-    const repo = new Repository({
-      url,
-      appName,
-      instanceId,
-      refreshInterval: 10,
-      // @ts-expect-error
-      bootstrapProvider: new DefaultBootstrapProvider({}),
-      storageProvider: new InMemStorageProvider(),
-    });
-
-    repo.on('error', (err) => {
-      t.truthy(err);
-      // eslint-disable-next-line max-len
-      t.is(
-        err.message,
-        // eslint-disable-next-line max-len
-        `${url}/client/features responded NOT_FOUND (404) which means your API url most likely needs correction. Stopping refresh of toggles`,
-      );
-      resolve();
-    });
-    repo.start();
-  }));
 
 test('should handle 304 as silent ok', (t) => {
   t.plan(0);
@@ -955,6 +928,55 @@ test('bootstrap should not override load backup-file', async (t) => {
   // @ts-expect-error
   t.is(repo.getToggle('feature-backup').enabled, true);
 });
+
+// Skipped because make-fetch-happens actually automatically retries two extra times on 404
+// with a timeout of 1000, this makes us have to wait up to 3 seconds for a single test to succeed
+// eslint-disable-next-line max-len
+test.skip('Failing two times and then succeed should decrease interval to 2 times initial interval', async (t) => {
+  const url = 'http://unleash-test-fail5times.app';
+  nock(url).persist().get('/client/features').reply(404);
+  const repo = new Repository({
+    url,
+    appName,
+    instanceId,
+    refreshInterval: 10,
+    // @ts-expect-error
+    bootstrapProvider: new DefaultBootstrapProvider({}),
+    storageProvider: new InMemStorageProvider(),
+  });
+  await repo.fetch();
+  t.is(1, repo.getFailures());
+  t.is(20, repo.nextFetch());
+  await repo.fetch();
+  t.is(2, repo.getFailures());
+  t.is(30, repo.nextFetch());
+  nock.cleanAll();
+  nock(url)
+    .persist()
+    .get('/client/features')
+    .reply(200, {
+      version: 2,
+      features: [
+        {
+          name: 'feature-backup',
+          enabled: true,
+          strategies: [
+            {
+              name: 'default',
+            },
+            {
+              name: 'backup',
+            },
+          ],
+        },
+      ],
+    });
+
+  await repo.fetch();
+  t.is(1, repo.getFailures());
+  t.is(20, repo.nextFetch());
+});
+
 // Skipped because make-fetch-happens actually automatically retries two extra times on 429
 // with a timeout of 1000, this makes us have to wait up to 3 seconds for a single test to succeed
 // eslint-disable-next-line max-len