Skip to content

consume getJson function for better error messages #110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ jobs:
- run: npm ci
- run: npm run build
- run: npm run format-check
- run: npm run pack
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

npm run build now runs ncc

- run: npm test
- name: Verify no unstaged changes
if: runner.os != 'windows'
Expand Down
2 changes: 1 addition & 1 deletion __tests__/verify-no-unstaged-changes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ if [[ "$(git status --porcelain)" != "" ]]; then
echo ----------------------------------------
echo Troubleshooting
echo ----------------------------------------
echo "::error::Unstaged changes detected. Locally try running: git clean -ffdx && npm ci && npm run all"
echo "::error::Unstaged changes detected. Locally try running: git clean -ffdx && npm ci && npm run pre-checkin"
exit 1
fi
121 changes: 116 additions & 5 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10602,6 +10602,15 @@ var HttpCodes;
HttpCodes[HttpCodes["ServiceUnavailable"] = 503] = "ServiceUnavailable";
HttpCodes[HttpCodes["GatewayTimeout"] = 504] = "GatewayTimeout";
})(HttpCodes = exports.HttpCodes || (exports.HttpCodes = {}));
var Headers;
(function (Headers) {
Headers["Accept"] = "accept";
Headers["ContentType"] = "content-type";
})(Headers = exports.Headers || (exports.Headers = {}));
var MediaTypes;
(function (MediaTypes) {
MediaTypes["ApplicationJson"] = "application/json";
})(MediaTypes = exports.MediaTypes || (exports.MediaTypes = {}));
/**
* Returns the proxy URL, depending upon the supplied url and proxy environment variables.
* @param serverUrl The server URL where the request will be sent. For example, https://api.github.com
Expand Down Expand Up @@ -10700,6 +10709,36 @@ class HttpClient {
sendStream(verb, requestUrl, stream, additionalHeaders) {
return this.request(verb, requestUrl, stream, additionalHeaders);
}
/**
* Gets a typed object from an endpoint
* Be aware that not found returns a null. Other errors (4xx, 5xx) reject the promise
*/
async getJson(requestUrl, additionalHeaders = {}) {
additionalHeaders[Headers.Accept] = this._getExistingOrDefaultHeader(additionalHeaders, Headers.Accept, MediaTypes.ApplicationJson);
let res = await this.get(requestUrl, additionalHeaders);
return this._processResponse(res, this.requestOptions);
}
async postJson(requestUrl, obj, additionalHeaders = {}) {
let data = JSON.stringify(obj, null, 2);
additionalHeaders[Headers.Accept] = this._getExistingOrDefaultHeader(additionalHeaders, Headers.Accept, MediaTypes.ApplicationJson);
additionalHeaders[Headers.ContentType] = this._getExistingOrDefaultHeader(additionalHeaders, Headers.ContentType, MediaTypes.ApplicationJson);
let res = await this.post(requestUrl, data, additionalHeaders);
return this._processResponse(res, this.requestOptions);
}
async putJson(requestUrl, obj, additionalHeaders = {}) {
let data = JSON.stringify(obj, null, 2);
additionalHeaders[Headers.Accept] = this._getExistingOrDefaultHeader(additionalHeaders, Headers.Accept, MediaTypes.ApplicationJson);
additionalHeaders[Headers.ContentType] = this._getExistingOrDefaultHeader(additionalHeaders, Headers.ContentType, MediaTypes.ApplicationJson);
let res = await this.put(requestUrl, data, additionalHeaders);
return this._processResponse(res, this.requestOptions);
}
async patchJson(requestUrl, obj, additionalHeaders = {}) {
let data = JSON.stringify(obj, null, 2);
additionalHeaders[Headers.Accept] = this._getExistingOrDefaultHeader(additionalHeaders, Headers.Accept, MediaTypes.ApplicationJson);
additionalHeaders[Headers.ContentType] = this._getExistingOrDefaultHeader(additionalHeaders, Headers.ContentType, MediaTypes.ApplicationJson);
let res = await this.patch(requestUrl, data, additionalHeaders);
return this._processResponse(res, this.requestOptions);
}
/**
* Makes a raw http request.
* All other methods such as get, post, patch, and request ultimately call this.
Expand Down Expand Up @@ -10883,6 +10922,14 @@ class HttpClient {
}
return lowercaseKeys(headers || {});
}
_getExistingOrDefaultHeader(additionalHeaders, header, _default) {
const lowercaseKeys = obj => Object.keys(obj).reduce((c, k) => (c[k.toLowerCase()] = obj[k], c), {});
let clientHeader;
if (this.requestOptions && this.requestOptions.headers) {
clientHeader = lowercaseKeys(this.requestOptions.headers)[header];
}
return additionalHeaders[header] || clientHeader || _default;
}
_getAgent(parsedUrl) {
let agent;
let proxyUrl = pm.getProxyUrl(parsedUrl);
Expand Down Expand Up @@ -10950,6 +10997,73 @@ class HttpClient {
const ms = ExponentialBackoffTimeSlice * Math.pow(2, retryNumber);
return new Promise(resolve => setTimeout(() => resolve(), ms));
}
static dateTimeDeserializer(key, value) {
if (typeof value === 'string') {
let a = new Date(value);
if (!isNaN(a.valueOf())) {
return a;
}
}
return value;
}
async _processResponse(res, options) {
return new Promise(async (resolve, reject) => {
const statusCode = res.message.statusCode;
const response = {
statusCode: statusCode,
result: null,
headers: {}
};
// not found leads to null obj returned
if (statusCode == HttpCodes.NotFound) {
resolve(response);
}
let obj;
let contents;
// get the result from the body
try {
contents = await res.readBody();
if (contents && contents.length > 0) {
if (options && options.deserializeDates) {
obj = JSON.parse(contents, HttpClient.dateTimeDeserializer);
}
else {
obj = JSON.parse(contents);
}
response.result = obj;
}
response.headers = res.message.headers;
}
catch (err) {
// Invalid resource (contents not json); leaving result obj null
}
// note that 3xx redirects are handled by the http layer.
if (statusCode > 299) {
let msg;
// if exception/error in body, attempt to get better error
if (obj && obj.message) {
msg = obj.message;
}
else if (contents && contents.length > 0) {
// it may be the case that the exception is in the body message as string
msg = contents;
}
else {
msg = "Failed request: (" + statusCode + ")";
}
let err = new Error(msg);
// attach statusCode and body obj (if available) to the error object
err['statusCode'] = statusCode;
if (response.result) {
err['result'] = response.result;
}
reject(err);
}
else {
resolve(response);
}
});
}
}
exports.HttpClient = HttpClient;

Expand Down Expand Up @@ -11979,7 +12093,6 @@ var __importStar = (this && this.__importStar) || function (mod) {
Object.defineProperty(exports, "__esModule", { value: true });
// Load tempDirectory before it gets wiped by tool-cache
let tempDirectory = process.env['RUNNER_TEMPDIRECTORY'] || '';
const assert = __importStar(__webpack_require__(357));
const core = __importStar(__webpack_require__(470));
const hc = __importStar(__webpack_require__(539));
const io = __importStar(__webpack_require__(1));
Expand Down Expand Up @@ -12070,10 +12183,8 @@ function queryLatestMatch(versionSpec) {
allowRetries: true,
maxRetries: 3
});
let response = yield httpClient.get(dataUrl);
assert.ok(response.message.statusCode === 200, `Unexpected HTTP status code '${response.message.statusCode}'`);
let body = yield response.readBody();
let nodeVersions = JSON.parse(body);
let response = yield httpClient.getJson(dataUrl);
let nodeVersions = response.result || [];
nodeVersions.forEach((nodeVersion) => {
// ensure this version supports your os and platform
if (nodeVersion.files.indexOf(dataFileName) >= 0) {
Expand Down
26 changes: 13 additions & 13 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 3 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@
"description": "setup node action",
"main": "lib/setup-node.js",
"scripts": {
"build": "tsc",
"build": "tsc && ncc build",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

aligned with the pattern set by setup-ruby

"format": "prettier --write **/*.ts",
"format-check": "prettier --check **/*.ts",
"pack": "ncc build",
"test": "jest",
"all": "npm run build && npm run format && npm run pack && npm test"
"pre-checkin": "npm run format && npm run build && npm test"
},
"repository": {
"type": "git",
Expand All @@ -26,7 +25,7 @@
"dependencies": {
"@actions/core": "^1.2.2",
"@actions/github": "^1.1.0",
"@actions/http-client": "^1.0.3",
"@actions/http-client": "^1.0.6",
"@actions/io": "^1.0.2",
"@actions/tool-cache": "^1.3.1",
"semver": "^6.1.1"
Expand Down
9 changes: 2 additions & 7 deletions src/installer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,8 @@ async function queryLatestMatch(versionSpec: string): Promise<string> {
allowRetries: true,
maxRetries: 3
});
let response = await httpClient.get(dataUrl);
assert.ok(
response.message.statusCode === 200,
`Unexpected HTTP status code '${response.message.statusCode}'`
);
let body = await response.readBody();
let nodeVersions = JSON.parse(body) as INodeVersion[];
let response = await httpClient.getJson<INodeVersion[]>(dataUrl);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

getJson now checks for errors, so i removed the error checking code here

let nodeVersions = response.result || [];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

|| [] is to satisfy typescript. Shouldnt be null since getJson checks for errors

nodeVersions.forEach((nodeVersion: INodeVersion) => {
// ensure this version supports your os and platform
if (nodeVersion.files.indexOf(dataFileName) >= 0) {
Expand Down