Skip to content

Commit

Permalink
More error checking and casing (#1799)
Browse files Browse the repository at this point in the history
* Translate GH's 403 to 503
* If/when the dep gets updated, gracefully handle GH's 403 as much as possible at authenticated and non-authenticated requests.
* The *@octokit/auth-token* dep returns lower casings for `authentication` and `basic`... not sure if matching this makes a difference but today I've been getting 45 maximum on unauthenticated GH rate limiting with this. Post #1729

Applies to #1705 #37

Auto-merge
  • Loading branch information
Martii authored Apr 17, 2021
1 parent 2706c67 commit 66c9082
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 22 deletions.
96 changes: 75 additions & 21 deletions controllers/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -1251,12 +1251,29 @@ exports.userGitHubRepoListPage = function (aReq, aRes, aNext) {
}

function asyncComplete(aErr) {
var msg = null;

if (aErr) {
console.error(aErr);
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: 'Server Error'
});
switch (aErr.code) {
case 403:
try {
msg = JSON.parse(aErr.message);
} catch (aE) {
msg = { message: aErr.message };
}
console.warn(msg.message);
statusCodePage(aReq, aRes, aNext, {
statusCode: 503,
statusMessage: 'Service unavailable. Please check back later.'
});
break;
default:
console.error(aErr);
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: 'Server Error'
});
}
return;
}

Expand Down Expand Up @@ -1378,8 +1395,29 @@ exports.userGitHubRepoPage = function (aReq, aRes, aNext) {
}

function asyncComplete(aErr) {
var msg = null;

if (aErr) {
aNext();
switch (aErr.code) {
case 403:
try {
msg = JSON.parse(aErr.message);
} catch (aE) {
msg = { message: aErr.message };
}
console.warn(msg.message);
statusCodePage(aReq, aRes, aNext, {
statusCode: 503,
statusMessage: 'Service unavailable. Please check back later.'
});
break;
default:
console.error(aErr);
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
statusMessage: 'Server Error'
});
}
return;
}

Expand Down Expand Up @@ -1411,7 +1449,7 @@ exports.userGitHubRepoPage = function (aReq, aRes, aNext) {
if (process.env.DISABLE_SCRIPT_IMPORT === 'true') {
statusCodePage(aReq, aRes, aNext, {
statusCode: 503,
statusMessage: 'Service unavailable. Please check back later'
statusMessage: 'Service unavailable. Please check back later.'
});
return;
}
Expand Down Expand Up @@ -1715,6 +1753,7 @@ exports.userGitHubImportScriptPage = function (aReq, aRes, aNext) {
], function (aErr) {
var script = null;
var code = null;
var msg = null;

if (aErr) {
code = (aErr instanceof statusError ? aErr.status.code : aErr.code);
Expand All @@ -1727,20 +1766,35 @@ exports.userGitHubImportScriptPage = function (aReq, aRes, aNext) {
}

if (!(aErr instanceof String)) {
statusCodePage(aReq, aRes, aNext, {
statusCode: (aErr instanceof statusError ? aErr.status.code : aErr.code),
statusMessage: (aErr instanceof statusError ? aErr.status.message : aErr.message),
isCustomView: true,
statusData: {
isGHImport: true,
utf_pathname: githubPathName,
utf_pathext: githubPathExt,
user: encodeURIComponent(githubUserId),
repo: encodeURIComponent(githubRepoName),
default_branch: encodeURIComponent(githubDefaultBranch),
path: encodeURIComponent(githubBlobPath)
}
});
switch (aErr.code) { // NOTE: Important to test for GH 403 vs potential OUJS 403
case 403:
try {
msg = JSON.parse(aErr.message);
} catch (aE) {
msg = { message: aErr.message };
}
console.warn(msg.message);
statusCodePage(aReq, aRes, aNext, {
statusCode: 503,
statusMessage: 'Service unavailable. Please check back later.'
});
break;
default:
statusCodePage(aReq, aRes, aNext, {
statusCode: (aErr instanceof statusError ? aErr.status.code : aErr.code),
statusMessage: (aErr instanceof statusError ? aErr.status.message : aErr.message),
isCustomView: true,
statusData: {
isGHImport: true,
utf_pathname: githubPathName,
utf_pathext: githubPathExt,
user: encodeURIComponent(githubUserId),
repo: encodeURIComponent(githubRepoName),
default_branch: encodeURIComponent(githubDefaultBranch),
path: encodeURIComponent(githubBlobPath)
}
});
}
} else {
statusCodePage(aReq, aRes, aNext, {
statusCode: 500,
Expand Down
2 changes: 1 addition & 1 deletion libs/repoManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function fetchJSON(aPath, aCallback) {
var encodedAuth = Buffer.from(`${clientId}:${clientKey}`).toString('base64');
var opts = {
headers: {
Authorization: `Basic ${encodedAuth}`
authorization: `basic ${encodedAuth}`
}
};
fetchRaw('api.github.com', aPath, function (aBufs) {
Expand Down

1 comment on commit 66c9082

@Martii
Copy link
Member Author

@Martii Martii commented on 66c9082 Apr 17, 2021

Choose a reason for hiding this comment

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

authentication authorization rather in commit summary

Please sign in to comment.