Skip to content

Commit

Permalink
fix: handle html server response
Browse files Browse the repository at this point in the history
  • Loading branch information
@shetzel
shetzel committed Jan 8, 2024
1 parent 7253cec commit 90d025d
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 13 deletions.
34 changes: 24 additions & 10 deletions src/deviceOauthService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import Transport from 'jsforce/lib/transport';
import { AsyncCreatable, Duration, parseJsonMap } 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';
Expand Down Expand Up @@ -39,6 +39,12 @@ export interface DeviceCodePollingResponse extends JsonMap {
issued_at: string;
}

interface DeviceCodeAuthError extends SfError {
error: string;
error_description: string;
status: number;
}

async function wait(ms = 1000): Promise<void> {
return new Promise((resolve) => {
setTimeout(resolve, ms);
Expand All @@ -47,7 +53,15 @@ async function wait(ms = 1000): Promise<void> {

async function makeRequest<T extends JsonMap>(options: HttpRequest): Promise<T> {
const rawResponse = await new Transport().httpRequest(options);
const response = parseJsonMap<T>(rawResponse.body);
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);
}
if (response.error) {
const errorDescription = typeof response.error_description === 'string' ? response.error_description : '';
const error = typeof response.error === 'string' ? response.error : 'Unknown';
Expand Down Expand Up @@ -175,21 +189,21 @@ 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) {
const err = (
e instanceof SfError ? e.data : SfError.wrap(isString(e) ? e : 'unknown').data
) as DeviceCodeAuthError;
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 */
}
}

Expand Down
42 changes: 39 additions & 3 deletions test/unit/deviceOauthServiceTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -71,18 +73,42 @@ 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', 'SfError');
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.'
);
}
});
});

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);
Expand All @@ -96,10 +122,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)
Expand All @@ -119,6 +151,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');
}
Expand All @@ -132,6 +165,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);
Expand All @@ -146,10 +180,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);
Expand Down

3 comments on commit 90d025d

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger Benchmarks - ubuntu-latest

Benchmark suite Current: 90d025d Previous: 1bb300b Ratio
Child logger creation 470001 ops/sec (±1.38%) 453953 ops/sec (±2.08%) 0.97
Logging a string on root logger 843426 ops/sec (±7.78%) 716372 ops/sec (±9.23%) 0.85
Logging an object on root logger 622345 ops/sec (±13.12%) 584520 ops/sec (±8.87%) 0.94
Logging an object with a message on root logger 23761 ops/sec (±184.91%) 10433 ops/sec (±200.85%) 0.44
Logging an object with a redacted prop on root logger 474599 ops/sec (±9.10%) 374209 ops/sec (±13.57%) 0.79
Logging a nested 3-level object on root logger 27204 ops/sec (±182.15%) 368413 ops/sec (±7.68%) 13.54

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Logger Benchmarks - ubuntu-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 90d025d Previous: 1bb300b Ratio
Logging a nested 3-level object on root logger 27204 ops/sec (±182.15%) 368413 ops/sec (±7.68%) 13.54

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger Benchmarks - windows-latest

Benchmark suite Current: 90d025d Previous: 1bb300b Ratio
Child logger creation 339649 ops/sec (±0.47%) 332891 ops/sec (±0.53%) 0.98
Logging a string on root logger 829622 ops/sec (±7.98%) 827720 ops/sec (±7.10%) 1.00
Logging an object on root logger 569402 ops/sec (±7.41%) 598338 ops/sec (±6.19%) 1.05
Logging an object with a message on root logger 7007 ops/sec (±201.14%) 8222 ops/sec (±201.49%) 1.17
Logging an object with a redacted prop on root logger 456951 ops/sec (±7.14%) 473796 ops/sec (±6.60%) 1.04
Logging a nested 3-level object on root logger 322977 ops/sec (±5.76%) 323221 ops/sec (±6.30%) 1.00

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.