From ea7ad36c65231b9a289c19f5dd185ca5bcf9c735 Mon Sep 17 00:00:00 2001
From: Steve Hetzel <shetzel@salesforce.com>
Date: Tue, 9 Jan 2024 10:20:03 -0700
Subject: [PATCH] fix: address review comments

---
 messages/auth.md                    |  4 ++++
 src/deviceOauthService.ts           | 34 ++++++++++++++---------------
 test/unit/deviceOauthServiceTest.ts |  6 ++---
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/messages/auth.md b/messages/auth.md
index 3e390daadd..6e866202b8 100644
--- a/messages/auth.md
+++ b/messages/auth.md
@@ -24,6 +24,10 @@ Invalid request method: %s
 
 Invalid request uri: %s
 
+# error.HttpApi
+
+HTTP response contains html content. Check that the org exists and can be reached.
+
 # pollingTimeout
 
 The device authorization request timed out. After executing force:auth:device:login, you must approve access to the device within 10 minutes. This can happen if the URL wasn’t copied into the browser, login was not attempted, or the 2FA process was not completed within 10 minutes. Request authorization again.
diff --git a/src/deviceOauthService.ts b/src/deviceOauthService.ts
index 27223d53b1..6f18cf2d35 100644
--- a/src/deviceOauthService.ts
+++ b/src/deviceOauthService.ts
@@ -8,7 +8,7 @@
 /* eslint-disable @typescript-eslint/ban-types */
 
 import Transport from 'jsforce/lib/transport';
-import { AsyncCreatable, Duration, parseJsonMap } from '@salesforce/kit';
+import { AsyncCreatable, Duration, parseJsonMap, sleep } from '@salesforce/kit';
 import { HttpRequest, OAuth2Config } from 'jsforce';
 import { ensureString, isString, JsonMap, Nullable } from '@salesforce/ts-types';
 import * as FormData from 'form-data';
@@ -45,23 +45,17 @@ interface DeviceCodeAuthError extends SfError {
   status: number;
 }
 
-async function wait(ms = 1000): Promise<void> {
-  return new Promise((resolve) => {
-    setTimeout(resolve, ms);
-  });
-}
-
 async function makeRequest<T extends JsonMap>(options: HttpRequest): Promise<T> {
   const rawResponse = await new Transport().httpRequest(options);
-  let response: T;
+
   if (rawResponse?.headers && rawResponse.headers['content-type'] === 'text/html') {
-    response = {
-      error: 'HttpApiError',
-      error_description: `HTTP response contains html content. Check that the org exists and can be reached. Response:\n${rawResponse.body}`,
-    } as unknown as T;
-  } else {
-    response = parseJsonMap<T>(rawResponse.body);
+    const htmlResponseError = messages.createError('error.HttpApi');
+    htmlResponseError.setData(rawResponse.body);
+    throw htmlResponseError;
   }
+
+  const response = parseJsonMap<T>(rawResponse.body);
+
   if (response.error) {
     const errorDescription = typeof response.error_description === 'string' ? response.error_description : '';
     const error = typeof response.error === 'string' ? response.error : 'Unknown';
@@ -190,9 +184,13 @@ export class DeviceOauthService extends AsyncCreatable<OAuth2Config> {
     try {
       return await makeRequest<DeviceCodePollingResponse>(httpRequest);
     } catch (e: unknown) {
-      const err = (
-        e instanceof SfError ? e.data : SfError.wrap(isString(e) ? e : 'unknown').data
-      ) as DeviceCodeAuthError;
+      if (e instanceof SfError && e.name === 'HttpApiError') {
+        throw e;
+      }
+
+      const err = (e instanceof SfError ? e.data : SfError.wrap(isString(e) ? e : 'unknown').data) as
+        | DeviceCodeAuthError
+        | undefined;
       if (err?.error && err?.status === 400 && err?.error === 'authorization_pending') {
         // do nothing because we're still waiting
       } else {
@@ -226,7 +224,7 @@ export class DeviceOauthService extends AsyncCreatable<OAuth2Config> {
       } else {
         this.logger.debug(`waiting ${interval} ms...`);
         // eslint-disable-next-line no-await-in-loop
-        await wait(interval);
+        await sleep(interval);
         this.pollingCount += 1;
       }
     }
diff --git a/test/unit/deviceOauthServiceTest.ts b/test/unit/deviceOauthServiceTest.ts
index c2f2248457..fc0bd9ce73 100644
--- a/test/unit/deviceOauthServiceTest.ts
+++ b/test/unit/deviceOauthServiceTest.ts
@@ -92,12 +92,10 @@ describe('DeviceOauthService', () => {
         await service.requestDeviceLogin();
         expect(true).to.be.false;
       } catch (err) {
-        expect(err).to.have.property('name', 'SfError');
+        expect(err).to.have.property('name', 'HttpApiError');
         expect(err)
           .to.have.property('message')
-          .and.contain(
-            'Request Failed: HttpApiError HTTP response contains html content. Check that the org exists and can be reached.'
-          );
+          .and.contain('HTTP response contains html content. Check that the org exists and can be reached.');
       }
     });
   });