[BUGFIX] ensure handleREsponse always called in rest adapter

packages/-ember-data/tests/integration/adapter/handle-response-test.js

@@ -1,7 +1,6 @@
 import { resolve, reject } from 'rsvp';
 import { setupTest } from 'ember-qunit';
 import { module, test } from 'qunit';
-import { DEBUG } from '@glimmer/env';
 import JSONAPIAdapter from '@ember-data/adapter/json-api';
 import AdapterError from '@ember-data/adapter/error';
 import JSONAPISerializer from 'ember-data/serializers/json-api';
@@ -100,7 +99,7 @@ module('integration/adapter/handle-response', function(hooks) {
       text() {
         return resolve(JSON.stringify(samplePayload));
       },
-    }
+    };
 
     let fetchResponse = {
       ok: true,
@@ -190,16 +189,11 @@ module('integration/adapter/handle-response', function(hooks) {
 
     this.owner.register('adapter:application', TestAdapter);
 
-    if (DEBUG) {
-      await assert.expectAssertion(async () => {
-        await this.store.findAll('person');
-      }, /Assertion Failed: You made a 'findAll' request for 'person' records, but the adapter's response did not have any data/);
-    } else {
-      try {
-        await this.store.findAll('person');
-      } catch {
-        assert.ok(true, 'promise rejected');
-      }
+    try {
+      await this.store.findAll('person');
+      assert.ok(false, 'promise should reject');
+    } catch {
+      assert.ok(true, 'promise rejected');
     }
 
     assert.equal(handleResponseCalled, 1, 'handle response is called');
@@ -253,16 +247,11 @@ module('integration/adapter/handle-response', function(hooks) {
 
     this.owner.register('adapter:application', TestAdapter);
 
-    if (DEBUG) {
-      await assert.expectAssertion(async () => {
-        await this.store.findAll('person');
-      }, /Assertion Failed: You made a 'findAll' request for 'person' records, but the adapter's response did not have any data/);
-    } else {
-      try {
-        await this.store.findAll('person');
-      } catch {
-        assert.ok(true, 'promise rejected');
-      }
+    try {
+      await this.store.findAll('person');
+      assert.ok(false, 'promise should reject');
+    } catch {
+      assert.ok(true, 'promise rejected');
     }
 
     assert.equal(handleResponseCalled, 1, 'handle response is called');

packages/-ember-data/tests/unit/utils/determine-body-promise-test.js

@@ -0,0 +1,75 @@
+// Copied from ember-fetch addon
+
+import { module, test } from 'qunit';
+import { resolve } from 'rsvp';
+import { determineBodyPromise } from '@ember-data/adapter/-private';
+
+class Response {
+  ok = true;
+
+  constructor(text, options) {
+    this.status = options.status;
+    this._text = text;
+  }
+
+  text() {
+    return resolve(this._text);
+  }
+}
+
+module('Unit | determineBodyPromise', function() {
+  test('determineBodyResponse returns the body when it is present', function(assert) {
+    assert.expect(1);
+
+    const response = new Response('{"data": "foo"}', { status: 200 });
+    const bodyPromise = determineBodyPromise(response, {});
+
+    return bodyPromise.then(body => {
+      assert.deepEqual(body, { data: 'foo' });
+    });
+  });
+
+  test('determineBodyResponse returns the body even if it is not json', function(assert) {
+    assert.expect(1);
+
+    const response = new Response('this is not json', { status: 200 });
+    const bodyPromise = determineBodyPromise(response, {});
+
+    return bodyPromise.then(body => {
+      assert.deepEqual(body, 'this is not json');
+    });
+  });
+
+  test('determineBodyResponse returns undefined when the http status code is 204', function(assert) {
+    assert.expect(1);
+
+    const response = new Response(null, { status: 204 });
+    const bodyPromise = determineBodyPromise(response, {});
+
+    return bodyPromise.then(body => {
+      assert.deepEqual(body, undefined);
+    });
+  });
+
+  test('determineBodyResponse returns undefined when the http status code is 205', function(assert) {
+    assert.expect(1);
+
+    const response = new Response(null, { status: 205 });
+    const bodyPromise = determineBodyPromise(response, {});
+
+    return bodyPromise.then(body => {
+      assert.deepEqual(body, undefined);
+    });
+  });
+
+  test("determineBodyResponse returns undefined when the request method is 'HEAD'", function(assert) {
+    assert.expect(1);
+
+    const response = new Response(null, { status: 200 });
+    const bodyPromise = determineBodyPromise(response, { method: 'HEAD' });
+
+    return bodyPromise.then(body => {
+      assert.deepEqual(body, undefined);
+    });
+  });
+});

packages/adapter/addon/-private/utils/determine-body-promise.ts

@@ -1,41 +1,60 @@
 import { warn } from '@ember/debug';
 import { DEBUG } from '@glimmer/env';
+import { resolve } from 'rsvp';
 
 /*
  * Function that always attempts to parse the response as json, and if an error is thrown,
  * returns `undefined` if the response is successful and has a status code of 204 (No Content),
  * or 205 (Reset Content) or if the request method was 'HEAD', and the plain payload otherwise.
  */
-export function determineBodyPromise(
+function _determineBodyPromise(
   response: Response,
-  requestData: JQueryAjaxSettings
+  requestData: JQueryAjaxSettings,
+  payload: string | object | undefined
 ): Promise<object | string | undefined> {
-  return response.text().then(function(payload) {
-    let ret: string | object | undefined = payload;
-    try {
-      ret = JSON.parse(payload);
-    } catch (error) {
-      if (!(error instanceof SyntaxError)) {
-        throw error;
-      }
-      const status = response.status;
-      if (response.ok && (status === 204 || status === 205 || requestData.method === 'HEAD')) {
-        ret = undefined;
-      } else {
-        if (DEBUG) {
-          let message = `The server returned an empty string for ${requestData.method} ${requestData.url}, which cannot be parsed into a valid JSON. Return either null or {}.`;
-          if (payload === '') {
-            warn(message, true, {
-              id: 'ds.adapter.returned-empty-string-as-JSON',
-            });
-            throw error;
-          }
+  let ret: string | object | undefined = payload;
+  try {
+    if (payload === null) {
+      throw new SyntaxError('null');
+    }
+    ret = JSON.parse(payload as string);
+  } catch (error) {
+    if (!(error instanceof SyntaxError)) {
+      throw error;
+    }
+    const status = response.status;
+    if (response.ok && (status === 204 || status === 205 || requestData.method === 'HEAD')) {
+      ret = undefined;
+    } else {
+      if (DEBUG) {
+        let message = `The server returned an empty string for ${requestData.method} ${requestData.url}, which cannot be parsed into a valid JSON. Return either null or {}.`;
+        if (payload === '') {
+          warn(message, true, {
+            id: 'ds.adapter.returned-empty-string-as-JSON',
+          });
+          throw error;
         }
-
-        // eslint-disable-next-line no-console
-        console.warn('This response was unable to be parsed as json.', payload);
       }
+
+      // eslint-disable-next-line no-console
+      console.warn('This response was unable to be parsed as json.', payload);
+    }
+  }
+  return resolve(ret);
+}
+
+export function determineBodyPromise(
+  response: Response,
+  requestData: JQueryAjaxSettings
+): Promise<object | string | undefined> {
+  // response.text() may resolve or reject
+  // it is a native promise, may not have finally
+  return response.text().then(
+    function(payload) {
+      return _determineBodyPromise(response, requestData, payload);
+    },
+    function(payload) {
+      return _determineBodyPromise(response, requestData, payload);
     }
-    return ret;
-  });
+  );
 }

packages/adapter/addon/rest.js

@@ -999,20 +999,30 @@ const RESTAdapter = Adapter.extend(BuildURLMixin, {
     let hash = adapter.ajaxOptions(url, type, options);
 
     if (useFetch) {
-      return this._fetchRequest(hash)
-        .then(response => {
-          return RSVP.hash({
-            response,
-            payload: determineBodyPromise(response, requestData),
-          });
-        })
-        .then(({ response, payload }) => {
-          if (response.ok) {
-            return fetchSuccessHandler(adapter, payload, response, requestData);
-          } else {
-            throw fetchErrorHandler(adapter, payload, response, null, requestData);
-          }
-        });
+      return (
+        this._fetchRequest(hash)
+          // fetch sometimes rejects and sometimes resolves with errors
+          .catch(response => {
+            response.ok = false;
+            let handlePayload = function(payload) {
+              return Promise.reject(fetchErrorHandler(adapter, payload, response, null, requestData));
+            };
+            return determineBodyPromise(response, requestData).then(handlePayload, handlePayload);
+          })
+          .then(response => {
+            return RSVP.hash({
+              response,
+              payload: determineBodyPromise(response, requestData),
+            });
+          })
+          .then(({ response, payload }) => {
+            if (response.ok) {
+              return fetchSuccessHandler(adapter, payload, response, requestData);
+            } else {
+              throw fetchErrorHandler(adapter, payload, response, null, requestData);
+            }
+          })
+      );
     }
 
     return new Promise(function(resolve, reject) {