Merge pull request #1015 from forcedotcom/sh/handle-html-response

fix: handle html server response

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.

src/deviceOauthService.ts

@@ -8,9 +8,9 @@
 /* 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, JsonMap, Nullable } from '@salesforce/ts-types';
+import { ensureString, isString, JsonMap, Nullable } from '@salesforce/ts-types';
 import * as FormData from 'form-data';
 import { Logger } from './logger/logger';
 import { AuthInfo, DEFAULT_CONNECTED_APP_INFO } from './org/authInfo';
@@ -39,15 +39,23 @@ export interface DeviceCodePollingResponse extends JsonMap {
   issued_at: string;
 }
 
-async function wait(ms = 1000): Promise<void> {
-  return new Promise((resolve) => {
-    setTimeout(resolve, ms);
-  });
+interface DeviceCodeAuthError extends SfError {
+  error: string;
+  error_description: string;
+  status: number;
 }
 
 async function makeRequest<T extends JsonMap>(options: HttpRequest): Promise<T> {
   const rawResponse = await new Transport().httpRequest(options);
+
+  if (rawResponse?.headers?.['content-type'] === 'text/html') {
+    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';
@@ -175,21 +183,25 @@ export class DeviceOauthService extends AsyncCreatable<OAuth2Config> {
     );
     try {
       return await makeRequest<DeviceCodePollingResponse>(httpRequest);
-    } catch (e) {
-      /* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/restrict-template-expressions */
-      const err: any = (e as SfError).data;
-      if (err.error && err.status === 400 && err.error === 'authorization_pending') {
+    } catch (e: unknown) {
+      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 {
-        if (err.error && err.error_description) {
+        if (err?.error && err?.error_description) {
           this.logger.error(`Polling error: ${err.error}: ${err.error_description}`);
         } else {
           this.logger.error('Unknown Polling Error:');
-          this.logger.error(err);
+          this.logger.error(err ?? e);
         }
-        throw err;
+        throw err ?? e;
       }
-      /* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/restrict-template-expressions */
     }
   }
 
@@ -212,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;
       }
     }

test/unit/deviceOauthServiceTest.ts

@@ -33,6 +33,8 @@ const devicePollingResponse = {
   issued_at: '1234',
 };
 
+const htmlResponse = '<html><body>Server down for maintenance</body></html>';
+
 type UnknownError = {
   error: string;
   status: number;
@@ -71,18 +73,40 @@ describe('DeviceOauthService', () => {
   describe('requestDeviceLogin', () => {
     it('should return the device code response', async () => {
       stubMethod($$.SANDBOX, Transport.prototype, 'httpRequest').returns(
-        Promise.resolve({ body: JSON.stringify(deviceCodeResponse) })
+        Promise.resolve({
+          body: JSON.stringify(deviceCodeResponse),
+          headers: { 'content-type': 'application/json;charset=UTF-8' },
+        })
       );
       const service = await DeviceOauthService.create({});
       const login = await service.requestDeviceLogin();
       expect(login).to.deep.equal(deviceCodeResponse);
     });
+
+    it('should handle HTML response with proper error', async () => {
+      stubMethod($$.SANDBOX, Transport.prototype, 'httpRequest').returns(
+        Promise.resolve({ body: htmlResponse, headers: { 'content-type': 'text/html' } })
+      );
+      const service = await DeviceOauthService.create({});
+      try {
+        await service.requestDeviceLogin();
+        expect(true).to.be.false;
+      } catch (err) {
+        expect(err).to.have.property('name', 'HttpApiError');
+        expect(err)
+          .to.have.property('message')
+          .and.contain('HTTP response contains html content. Check that the org exists and can be reached.');
+      }
+    });
   });
 
   describe('awaitDeviceApproval', () => {
     it('should return the device polling response', async () => {
       stubMethod($$.SANDBOX, Transport.prototype, 'httpRequest').returns(
-        Promise.resolve({ body: JSON.stringify(devicePollingResponse) })
+        Promise.resolve({
+          body: JSON.stringify(devicePollingResponse),
+          headers: { 'content-type': 'application/json;charset=UTF-8' },
+        })
       );
       const service = await DeviceOauthService.create({});
       const approval = await service.awaitDeviceApproval(deviceCodeResponse);
@@ -96,10 +120,16 @@ describe('DeviceOauthService', () => {
           Promise.resolve({
             statusCode: 400,
             body: JSON.stringify({ error: 'authorization_pending' }),
+            headers: { 'content-type': 'application/json;charset=UTF-8' },
           })
         )
         .onSecondCall()
-        .returns(Promise.resolve({ body: JSON.stringify(devicePollingResponse) }));
+        .returns(
+          Promise.resolve({
+            body: JSON.stringify(devicePollingResponse),
+            headers: { 'content-type': 'application/json;charset=UTF-8' },
+          })
+        );
       const shouldContinuePollingStub = stubMethod($$.SANDBOX, DeviceOauthService.prototype, 'shouldContinuePolling')
         .onFirstCall()
         .returns(true)
@@ -119,6 +149,7 @@ describe('DeviceOauthService', () => {
       service.pollingCount = DeviceOauthService.POLLING_COUNT_MAX + 1;
       try {
         await service.awaitDeviceApproval(deviceCodeResponse);
+        expect(true).to.be.false;
       } catch (err) {
         expect((err as Error).name).to.equal('PollingTimeoutError');
       }
@@ -132,6 +163,7 @@ describe('DeviceOauthService', () => {
       const service = await DeviceOauthService.create({});
       try {
         await service.awaitDeviceApproval(deviceCodeResponse);
+        expect(true).to.be.false;
       } catch (err) {
         // @ts-expect-error: because private member
         expect(service.pollingCount).to.equal(0);
@@ -146,10 +178,12 @@ describe('DeviceOauthService', () => {
           error: 'Invalid grant type',
           error_description: 'Invalid grant type',
         }),
+        headers: { 'content-type': 'application/json;charset=UTF-8' },
       }));
       const service = await DeviceOauthService.create({});
       try {
         await service.awaitDeviceApproval(deviceCodeResponse);
+        expect(true).to.be.false;
       } catch (err) {
         // @ts-expect-error: because private member
         expect(service.pollingCount).to.equal(0);