Skip to content

Commit

Permalink
Handle GHRateLimit expiration and server time inconsistency
Browse files Browse the repository at this point in the history
@bitwiseman
bitwiseman committed Nov 7, 2019
1 parent 7b7445b commit 1ecad70
Showing 23 changed files with 592 additions and 184 deletions.
157 changes: 78 additions & 79 deletions src/main/java/org/kohsuke/github/GHRateLimit.java
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package org.kohsuke.github;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.apache.commons.lang3.StringUtils;

import java.time.Clock;
import javax.annotation.Nonnull;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException;
@@ -17,6 +18,7 @@

/**
* Rate limit.
*
* @author Kohsuke Kawaguchi
*/
public class GHRateLimit {
@@ -49,118 +51,114 @@ public class GHRateLimit {
/**
* Remaining calls that can be made.
*/
private int remainingCount;
private final int remainingCount;

/**
* Allotted API call per hour.
*/
private int limitCount;
private final int limitCount;

/**
* The time at which the current rate limit window resets in UTC epoch seconds.
*/
private long resetEpochSeconds = -1;

/**
* String representation of the Date header from the response.
* If null, the value is ignored.
* Package private and effectively final.
*/
@CheckForNull
String updatedAt = null;
private final long resetEpochSeconds;

/**
* EpochSeconds time (UTC) at which this response was updated.
* Will be updated to match {@link this.updatedAt} if that is not null.
*/
private long updatedAtEpochSeconds = System.currentTimeMillis() / 1000L;
private final long createdAtEpochSeconds = System.currentTimeMillis() / 1000;

/**
* The calculated time at which the rate limit will reset.
* Only calculated if {@link #getResetDate} is called.
*/
@CheckForNull
private Date calculatedResetDate;
private Date resetDate = null;

/**
* Gets a placeholder instance that can be used when we fail to get one from the server.
*
* @return a GHRateLimit
*/
public static GHRateLimit getPlaceholder() {
GHRateLimit r = new GHRateLimit();
r.setLimit(1000000);
r.setRemaining(1000000);
long minute = 60L;
final long oneHour = 60L * 60L;
// This placeholder limit does not expire for a while
// This make it so that calling rateLimit() multiple times does not result in multiple request
r.setResetEpochSeconds(System.currentTimeMillis() / 1000L + minute);
GHRateLimit r = new GHRateLimit(1000000, 1000000, System.currentTimeMillis() / 1000L + oneHour);
return r;
}

/**
* Gets the remaining number of requests allowed before this connection will be throttled.
*
* @return an integer
*/
@JsonProperty("remaining")
public int getRemaining() {
return remainingCount;
@JsonCreator
public GHRateLimit(@JsonProperty("limit") int limit,
@JsonProperty("remaining") int remaining,
@JsonProperty("reset")long resetEpochSeconds) {
this(limit, remaining, resetEpochSeconds, null);
}

/**
* Sets the remaining number of requests allowed before this connection will be throttled.
*
* @param remaining an integer
*/
@JsonProperty("remaining")
void setRemaining(int remaining) {
public GHRateLimit(int limit, int remaining, long resetEpochSeconds, String updatedAt) {
this.limitCount = limit;
this.remainingCount = remaining;
this.resetEpochSeconds = resetEpochSeconds;
setUpdatedAt(updatedAt);

// Deprecated fields
this.remaining = remaining;
this.limit = limit;
this.reset = new Date(resetEpochSeconds);

}

/**
*
* @param updatedAt a string date in RFC 1123
*/
void setUpdatedAt(String updatedAt) {
long updatedAtEpochSeconds = createdAtEpochSeconds;
if (!StringUtils.isBlank(updatedAt)) {
try {
// Get the server date and reset data, will always return a time in GMT
updatedAtEpochSeconds = ZonedDateTime.parse(updatedAt, DateTimeFormatter.RFC_1123_DATE_TIME).toEpochSecond();
} catch (DateTimeParseException e) {
if (LOGGER.isLoggable(FINEST)) {
LOGGER.log(FINEST, "Malformed Date header value " + updatedAt, e);
}
}
}

long calculatedSecondsUntilReset = resetEpochSeconds - updatedAtEpochSeconds;

// This may seem odd but it results in an accurate or slightly pessimistic reset date
resetDate = new Date((createdAtEpochSeconds + calculatedSecondsUntilReset) * 1000);
}

/**
* Gets the total number of API calls per hour allotted for this connection.
* Gets the remaining number of requests allowed before this connection will be throttled.
*
* @return an integer
*/
@JsonProperty("limit")
public int getLimit() {
return limitCount;
public int getRemaining() {
return remainingCount;
}

/**
* Sets the total number of API calls per hour allotted for this connection.
* Gets the total number of API calls per hour allotted for this connection.
*
* @param limit an integer
* @return an integer
*/
@JsonProperty("limit")
void setLimit(int limit) {
this.limitCount = limit;
this.limit = limit;
public int getLimit() {
return limitCount;
}

/**
* Gets the time in epoch seconds when the rate limit will reset.
*
* @return a long
*/
@JsonProperty("reset")
public long getResetEpochSeconds() {
return resetEpochSeconds;
}

/**
* The time in epoch seconds when the rate limit will reset.
*
* @param resetEpochSeconds the reset time in epoch seconds
*/
@JsonProperty("reset")
void setResetEpochSeconds(long resetEpochSeconds) {
this.resetEpochSeconds = resetEpochSeconds;
this.reset = new Date(resetEpochSeconds);
}

/**
* Whether the rate limit reset date indicated by this instance is in the
*
@@ -171,35 +169,15 @@ public boolean isExpired() {
}

/**
* Calculates the date at which the rate limit will reset.
* If available, it uses the server time indicated by the Date response header to accurately
* calculate this date. If not, it uses the system time UTC.
* Returns the date at which the rate limit will reset.
*
* @return the calculated date at which the rate limit has or will reset.
*/
@SuppressFBWarnings(value = "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR",
justification = "The value comes from JSON deserialization")
@Nonnull
public Date getResetDate() {
if (calculatedResetDate == null) {
if (!StringUtils.isBlank(updatedAt)) {
// this is why we wait to calculate the reset date - it is expensive.
try {
// Get the server date and reset data, will always return a time in GMT
updatedAtEpochSeconds = ZonedDateTime.parse(updatedAt, DateTimeFormatter.RFC_1123_DATE_TIME).toEpochSecond();
} catch (DateTimeParseException e) {
if (LOGGER.isLoggable(FINEST)) {
LOGGER.log(FINEST, "Malformed Date header value " + updatedAt, e);
}
}
}

long calculatedSecondsUntilReset = resetEpochSeconds - updatedAtEpochSeconds;

// This may seem odd but it results in an accurate or slightly pessimistic reset date
calculatedResetDate = new Date((updatedAtEpochSeconds + calculatedSecondsUntilReset) * 1000);
}

return calculatedResetDate;
return new Date(resetDate.getTime());
}

@Override
@@ -211,5 +189,26 @@ public String toString() {
'}';
}


@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
GHRateLimit rateLimit = (GHRateLimit) o;
return getRemaining() == rateLimit.getRemaining() &&
getLimit() == rateLimit.getLimit() &&
getResetEpochSeconds() == rateLimit.getResetEpochSeconds() &&
getResetDate().equals(rateLimit.getResetDate());
}

@Override
public int hashCode() {
return Objects.hash(getRemaining(), getLimit(), getResetEpochSeconds(), getResetDate());
}

private static final Logger LOGGER = Logger.getLogger(Requester.class.getName());
}
21 changes: 9 additions & 12 deletions src/main/java/org/kohsuke/github/GitHub.java
View file
Original file line number Diff line number Diff line change
@@ -316,28 +316,25 @@ public void setConnector(HttpConnector connector) {
* Gets the current rate limit.
*/
public GHRateLimit getRateLimit() throws IOException {
GHRateLimit rateLimit;
try {
GHRateLimit rateLimit = retrieve().to("/rate_limit", JsonRateLimit.class).rate;
// Use the response date from the header
GHRateLimit lastRateLimit = lastRateLimit();
if (lastRateLimit != null) {
rateLimit.updatedAt = lastRateLimit.updatedAt;
}
return this.rateLimit = rateLimit;
rateLimit = retrieve().to("/rate_limit", JsonRateLimit.class).rate;
} catch (FileNotFoundException e) {
// GitHub Enterprise doesn't have the rate limit, so in that case
// return some big number that's not too big.
// see issue #78
GHRateLimit r = GHRateLimit.getPlaceholder();
return rateLimit = r;
rateLimit = GHRateLimit.getPlaceholder();
}

return this.rateLimit = rateLimit;

}

/*package*/ void updateRateLimit(@Nonnull GHRateLimit observed) {
synchronized (headerRateLimitLock) {
if (headerRateLimit == null
|| headerRateLimit.getResetDate().getTime() < observed.getResetDate().getTime()
|| headerRateLimit.getRemaining() > observed.getRemaining()) {
|| headerRateLimit.getRemaining() > observed.getRemaining()
|| headerRateLimit.getResetDate().getTime() < observed.getResetDate().getTime()) {
headerRateLimit = observed;
LOGGER.log(FINE, "Rate limit now: {0}", headerRateLimit);
}
@@ -366,7 +363,7 @@ public GHRateLimit lastRateLimit() {
@Nonnull
public GHRateLimit rateLimit() throws IOException {
synchronized (headerRateLimitLock) {
if (headerRateLimit != null) {
if (headerRateLimit != null && !headerRateLimit.isExpired()) {
return headerRateLimit;
}
}
40 changes: 20 additions & 20 deletions src/main/java/org/kohsuke/github/Requester.java
View file
Original file line number Diff line number Diff line change
@@ -343,59 +343,56 @@ public InputStream asStream(String tailApiUrl) throws IOException {
}

private void noteRateLimit(String tailApiUrl) {
// "/rate_limit" is free, but we want to always get header rate limit anyway.
// Specifically, in this method we get the accurate server Date for calculating reset time.

if (tailApiUrl.startsWith("/search")) {
// the search API uses a different rate limit
return;
}
String limit = uc.getHeaderField("X-RateLimit-Limit");
if (StringUtils.isBlank(limit)) {
String limitString = uc.getHeaderField("X-RateLimit-Limit");
if (StringUtils.isBlank(limitString)) {
// if we are missing a header, return fast
return;
}
String remaining = uc.getHeaderField("X-RateLimit-Remaining");
if (StringUtils.isBlank(remaining)) {
String remainingString = uc.getHeaderField("X-RateLimit-Remaining");
if (StringUtils.isBlank(remainingString)) {
// if we are missing a header, return fast
return;
}
String reset = uc.getHeaderField("X-RateLimit-Reset");
if (StringUtils.isBlank(reset)) {
String resetString = uc.getHeaderField("X-RateLimit-Reset");
if (StringUtils.isBlank(resetString)) {
// if we are missing a header, return fast
return;
}

GHRateLimit observed = new GHRateLimit();

// Date header can be missing or invalid, will be ignored later.
observed.updatedAt = uc.getHeaderField("Date");;

int limit, remaining;
long reset;
try {
observed.setLimit(Integer.parseInt(limit));
limit = Integer.parseInt(limitString);
} catch (NumberFormatException e) {
if (LOGGER.isLoggable(FINEST)) {
LOGGER.log(FINEST, "Malformed X-RateLimit-Limit header value " + limit, e);
LOGGER.log(FINEST, "Malformed X-RateLimit-Limit header value " + limitString, e);
}
return;
}
try {
observed.setRemaining(Integer.parseInt(remaining));

remaining = Integer.parseInt(remainingString);
} catch (NumberFormatException e) {
if (LOGGER.isLoggable(FINEST)) {
LOGGER.log(FINEST, "Malformed X-RateLimit-Remaining header value " + remaining, e);
LOGGER.log(FINEST, "Malformed X-RateLimit-Remaining header value " + remainingString, e);
}
return;
}
try {
observed.setResetEpochSeconds(Long.parseLong(reset));
reset = Long.parseLong(resetString);
} catch (NumberFormatException e) {
if (LOGGER.isLoggable(FINEST)) {
LOGGER.log(FINEST, "Malformed X-RateLimit-Reset header value " + reset, e);
LOGGER.log(FINEST, "Malformed X-RateLimit-Reset header value " + resetString, e);
}
return;
}

GHRateLimit observed = new GHRateLimit(limit, remaining, reset, uc.getHeaderField("Date"));

root.updateRateLimit(observed);
}

@@ -703,6 +700,9 @@ private <T> T setResponseHeaders(T readValue) {
}
} else if (readValue instanceof GHObject) {
setResponseHeaders((GHObject) readValue);
} else if (readValue instanceof JsonRateLimit) {
// if we're getting a GHRateLimit it needs the server date
((JsonRateLimit)readValue).rate.setUpdatedAt(uc.getHeaderField("Date"));
}
return readValue;
}
Loading

0 comments on commit 1ecad70

Please sign in to comment.