Skip to content
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

feat: rate limit requests using aperture #14

Merged
merged 10 commits into from
Dec 15, 2023
4 changes: 4 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ on:
pull_request:
branches: [ "main" ]

env:
APERTURE_SERVICE_ADDRESS: ${{ secrets.APERTURE_SERVICE_ADDRESS }}
APERTURE_API_KEY: ${{ secrets.APERTURE_API_KEY }}

jobs:
build:

Expand Down
6 changes: 3 additions & 3 deletions contributors.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import * as path from 'path';
import * as fs from 'fs';

import { makeRequest } from './network.js';
import { makeRequest, makeRequestWithRateLimit } from './network.js';

// Configurations (Optional)
// Repo owner that you want to analyze
Expand Down Expand Up @@ -39,7 +39,7 @@ export async function getAllRepos(owner=REPO_OWNER, options) {
GITHUB_REQUEST_OPTIONS.headers["Authorization"] = "token "+options.GITHUB_PERSONAL_TOKEN;
Copy link
Contributor

Choose a reason for hiding this comment

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

The previous comment about the potential side effects of modifying GITHUB_REQUEST_OPTIONS directly is still valid. Consider creating a new options object within the function scope to avoid potential side effects.

}
let url = `https://api.github.com/orgs/${owner}/repos?per_page=100&page=${pageNo}`;
const { res, data } = await makeRequest('GET', url, Object.assign({},GITHUB_REQUEST_OPTIONS));
const { res, data } = await makeRequestWithRateLimit('GET', url, Object.assign({},GITHUB_REQUEST_OPTIONS));
console.log("Repo list request finished");
console.log('HTTP status: ', res.statusCode);
// console.log(data)
Comment on lines 43 to 45
Copy link
Contributor

Choose a reason for hiding this comment

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

The previous comment about ensuring proper error handling in case the request fails or the status code indicates an error is still valid. Consider adding explicit error handling to manage different HTTP status codes and request failures.

Expand Down Expand Up @@ -67,7 +67,7 @@ export async function getAllRepos(owner=REPO_OWNER, options) {
export async function getRepoContributors(fullRepoName, pageNo = 1) {
let url = `https://api.github.com/repos/${fullRepoName}/contributors?per_page=100&page=${pageNo}`;
console.log(url);
const { res, data } = await makeRequest('GET', url, Object.assign({},GITHUB_REQUEST_OPTIONS));
const { res, data } = await makeRequestWithRateLimit('GET', url, Object.assign({},GITHUB_REQUEST_OPTIONS));
console.log("Contributors request finished for " + fullRepoName)
// console.log(data)
let dataJsonArray = JSON.parse(data);
Expand Down
96 changes: 89 additions & 7 deletions network.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,27 @@
import * as https from 'https';
import { ApertureClient } from "@fluxninja/aperture-js";


var apertureClient;

function getApertureClient(){
if(!apertureClient) {
apertureClient = new ApertureClient({
address: process.env.APERTURE_SERVICE_ADDRESS,
apiKey: process.env.APERTURE_API_KEY,
});
}
return apertureClient;
}

/**
* Sends an HTTPS request based on the specified method and options
* This function returns a Promise that resolves with the response and data received from the server
* @param {string} method - The HTTP method to use (e.g., 'GET', 'POST').
* @param {string} url - The URL to which the request is sent.
* @param {Object} [options] - The options for the request. This includes headers, request body (for POST/PUT), etc.
* @param {Object} [requestOptions] - The options for the request. This includes headers, request body (for POST/PUT), etc.
* @param {Object} [requestOptions.data] - The data that needs to be sent with the request, used for POST/PUT requests
* @param {Object} [requestOptions.headers] - The headers that needs to be sent with the request
* @returns {Promise<{res: https.IncomingMessage, data: string}>} A Promise that resolves with the response object and the body data as a string.
* @throws {Error} Throws an error if the request cannot be completed
*
Expand All @@ -21,13 +37,13 @@ import * as https from 'https';
* }
* }
* */
export async function makeRequest(method, url, options) {
export async function makeRequest(method, url, requestOptions) {
return new Promise((resolve, reject) => {
// Ensure options is an object and set the method
options = typeof options === 'object' ? options : {};
options.method = method;
requestOptions = typeof requestOptions === 'object' ? requestOptions : {};
requestOptions.method = method;

const req = https.request(url, options, res => {
const req = https.request(url, requestOptions, res => {
// Handle HTTP response stream
let data = '';
res.on('data', chunk => data += chunk);
Expand All @@ -40,10 +56,76 @@ export async function makeRequest(method, url, options) {
});

// Handle POST/PUT data if provided
if (options.data) {
req.write(options.data);
if (requestOptions.data) {
req.write(requestOptions.data);
Comment on lines +59 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

The previous comment about renaming requestOptions.data to requestOptions.body has not been addressed. This should be consistent with the JSDoc comments and the rest of the codebase.

- if (requestOptions.data) {
-     req.write(requestOptions.data);
+ if (requestOptions.body) {
+     req.write(requestOptions.body);

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if (requestOptions.data) {
req.write(requestOptions.data);
if (requestOptions.body) {
req.write(requestOptions.body);

}

req.end();
});
}


/**
* Sends an HTTPS request with rate limiting. The function checks if the request is allowed by the rate limiter,
* and if so, sends an HTTPS request based on the specified method and options. This function returns a Promise
* that resolves with the response and data received from the server or rejects if an error occurs or the rate limit is exceeded.
*
* @param {string} method - The HTTP method to use (e.g., 'GET', 'POST').
* @param {string} url - The URL to which the request is sent.
* @param {Object} [options] - The options for the request. This includes headers, request body (for POST/PUT), etc.
* @param {Object} [options.rateLimitOptions] - Options related to rate limits
* @param {Object} [options.data] - The data that needs to be sent with the request, used for POST/PUT requests
* @param {Object} [options.headers] - The headers that needs to be sent with the request
* @returns {Promise<{res: https.IncomingMessage, data: string}>} A Promise that resolves with the response object and the body data as a string.
* @throws {Error} Throws an error if the rate limit is exceeded or if an invalid argument is passed.
*
* @example
* // Example usage for a GET request within an async function
* async function getExample() {
* try {
* const { res, data } = await makeRequest('GET', 'https://example.com');
* console.log('Status Code:', res.statusCode);
* console.log('Body:', data);
* } catch (error) {
* console.error('Error:', error.message);
* }
* }
* */
export async function makeRequestWithRateLimit(method, url, options){
let flow;
try {
flow = await getApertureClient().startFlow("external-api", {
labels: {
url: url,
priority: 1,
tokens: 1,
workload: 'api.github'
},
grpcCallOptions: {
deadline: Date.now() + 300, // ms
},
});
} catch(err){
console.error("Aperture client for rate limiting is not setup correctly");
console.log("Make sure to setup set correct APERTURE_SERVICE_ADDRESS and APERTURE_API_KEY in environment variables");
console.log("Going to make request without rate limit");
} finally {
// Make request if allowed as per rate limit
// In case rate limiting is not setup properly, make request anyway
if (!flow || flow.shouldRun()) {
// Add business logic to process incoming request
console.log("Request accepted. Processing...");
const {res, data} = await makeRequest(...arguments)
return { res, data}
} else {
console.log("Request rate-limited. Try again later.");
if(flow) {
// Handle flow rejection
flow.setStatus(FlowStatus.Error);
}
}
if (flow) {
flow.end();
}
}
}
Loading