Skip to content

Commit

Permalink
fix: address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
shetzel committed Jan 9, 2024
1 parent a5e33f0 commit ea7ad36
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 22 deletions.
4 changes: 4 additions & 0 deletions messages/auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
34 changes: 16 additions & 18 deletions src/deviceOauthService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
}
Expand Down
6 changes: 2 additions & 4 deletions test/unit/deviceOauthServiceTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
});
});
Expand Down

3 comments on commit ea7ad36

@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: ea7ad36 Previous: a5e33f0 Ratio
Child logger creation 463839 ops/sec (±1.17%) 468681 ops/sec (±2.31%) 1.01
Logging a string on root logger 641764 ops/sec (±7.09%) 836635 ops/sec (±11.11%) 1.30
Logging an object on root logger 525488 ops/sec (±9.11%) 647514 ops/sec (±7.50%) 1.23
Logging an object with a message on root logger 20679 ops/sec (±184.78%) 30550 ops/sec (±182.22%) 1.48
Logging an object with a redacted prop on root logger 409247 ops/sec (±11.62%) 480037 ops/sec (±10.15%) 1.17
Logging a nested 3-level object on root logger 328455 ops/sec (±8.01%) 27310 ops/sec (±183.25%) 0.0831468542113836

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: ea7ad36 Previous: a5e33f0 Ratio
Child logger creation 327908 ops/sec (±1.03%) 316712 ops/sec (±0.79%) 0.97
Logging a string on root logger 814435 ops/sec (±4.22%) 733332 ops/sec (±6.00%) 0.90
Logging an object on root logger 652160 ops/sec (±5.73%) 602608 ops/sec (±6.08%) 0.92
Logging an object with a message on root logger 2854 ops/sec (±224.10%) 10477 ops/sec (±198.44%) 3.67
Logging an object with a redacted prop on root logger 488071 ops/sec (±9.23%) 499118 ops/sec (±5.30%) 1.02
Logging a nested 3-level object on root logger 335250 ops/sec (±7.65%) 332966 ops/sec (±6.22%) 0.99

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 - windows-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: ea7ad36 Previous: a5e33f0 Ratio
Logging an object with a message on root logger 2854 ops/sec (±224.10%) 10477 ops/sec (±198.44%) 3.67

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

Please sign in to comment.