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.

Sign up for GitHub

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

refactor: use more clear retryer error messages #3216

Merged
merged 1 commit into from
Sep 12, 2023
Merged

refactor: use more clear retryer error messages #3216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion src/common/retryer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { CustomError, logger } from "./utils.js";

// Script variables.

// Count the number of GitHub API tokens available.
const PATs = Object.keys(process.env).filter((key) =>
/PAT_\d*$/.exec(key),
).length;
const RETRIES = PATs ? PATs : 7;
const RETRIES = process.env.NODE_ENV === "test" ? 7 : PATs;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@qwerty541 Although this PR is a great addition, I think the process could have been better.NODE_ENV variable is needed. The vercel variables should be available locally when running vercel dev (see https://vercel.com/docs/projects/environment-variables#environments).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rickstaa I did it this way since this code was already used in another parts of projects, for example https://github.com/anuraghazra/github-readme-stats/blob/master/src/getStyles.js#L134. As i understand NODE_ENV is not vercel variable, it was not set on production or local development, it was set by jest framework only for testing purposes, see https://jestjs.io/docs/environment-variables#node_env

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool, let's merge this then 👍🏻.


/**
* @typedef {import("axios").AxiosResponse} AxiosResponse Axios response.
Expand All @@ -20,6 +22,9 @@ const RETRIES = PATs ? PATs : 7;
* @returns {Promise<T>} The response from the fetcher function.
*/
const retryer = async (fetcher, variables, retries = 0) => {
if (!RETRIES) {
throw new CustomError("No GitHub API tokens found", CustomError.NO_TOKENS);
}
if (retries > RETRIES) {
throw new CustomError("Maximum retries exceeded", CustomError.MAX_RETRY);
}
Expand Down
6 changes: 4 additions & 2 deletions src/common/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,9 @@ const CONSTANTS = {
};

const SECONDARY_ERROR_MESSAGES = {
MAX_RETRY:
"Please add an env variable called PAT_1 with your github token in vercel",
MAX_RETRY: "Downtime due to GitHub API rate limiting",
NO_TOKENS:
"Please add an env variable called PAT_1 with your GitHub API token in vercel",
USER_NOT_FOUND: "Make sure the provided username is not an organization",
GRAPHQL_ERROR: "Please try again later",
WAKATIME_USER_NOT_FOUND: "Make sure you have a public WakaTime profile",
Expand All @@ -340,6 +341,7 @@ class CustomError extends Error {
}

static MAX_RETRY = "MAX_RETRY";
static NO_TOKENS = "NO_TOKENS";
static USER_NOT_FOUND = "USER_NOT_FOUND";
static GRAPHQL_ERROR = "GRAPHQL_ERROR";
static WAKATIME_ERROR = "WAKATIME_ERROR";
Expand Down