-
Notifications
You must be signed in to change notification settings - Fork 75
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: Backoff based on HTTP statuses #537
Changes from 11 commits
5060750
39e51be
30c9dff
374c44d
1e598c6
204e19b
799a51c
a6df198
0549a2e
56d5908
404d610
75c83e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,8 @@ export default class Repository extends EventEmitter implements EventEmitter { | |
|
||
private headers?: CustomHeaders; | ||
|
||
private failures: number = 0; | ||
|
||
private customHeadersFunction?: CustomHeadersFunction; | ||
|
||
private timeout?: number; | ||
|
@@ -241,11 +243,92 @@ Message: ${err.message}`, | |
return obj; | ||
} | ||
|
||
getFailures(): number { | ||
return this.failures; | ||
} | ||
|
||
nextFetch(): number { | ||
return this.refreshInterval + this.failures * this.refreshInterval; | ||
} | ||
|
||
private backoff(): number { | ||
this.failures = Math.min(this.failures + 1, 10); | ||
return this.nextFetch(); | ||
} | ||
|
||
private countSuccess(): number { | ||
this.failures = Math.max(this.failures - 1, 0); | ||
return this.nextFetch(); | ||
} | ||
|
||
// Emits correct error message based on what failed, | ||
// and returns 0 as the next fetch interval (stop polling) | ||
private configurationError(url: string, statusCode: number): number { | ||
this.failures += 1; | ||
if (statusCode === 404) { | ||
this.emit( | ||
UnleashEvents.Error, | ||
new Error( | ||
// eslint-disable-next-line max-len | ||
`${url} responded NOT_FOUND (404) which means your API url most likely needs correction. Stopping refresh of toggles`, | ||
), | ||
); | ||
} else if (statusCode === 401 || statusCode === 403) { | ||
this.emit( | ||
UnleashEvents.Error, | ||
new Error( | ||
// eslint-disable-next-line max-len | ||
`${url} responded ${statusCode} which means your API key is not allowed to connect. Stopping refresh of toggles`, | ||
), | ||
); | ||
} | ||
return 0; | ||
} | ||
|
||
// We got a status code we know what to do with, so will log correct message | ||
// and return the new interval. | ||
private recoverableError(url: string, statusCode: number): number { | ||
let nextFetch = this.backoff(); | ||
if (statusCode === 429) { | ||
this.emit( | ||
UnleashEvents.Warn, | ||
// eslint-disable-next-line max-len | ||
`${url} responded TOO_MANY_CONNECTIONS (429). Backing off`, | ||
); | ||
} else if (statusCode === 500 || | ||
statusCode === 502 || | ||
statusCode === 503 || | ||
statusCode === 504) { | ||
this.emit( | ||
UnleashEvents.Warn, | ||
`${url} responded ${statusCode}. Backing off`, | ||
); | ||
} | ||
return nextFetch; | ||
} | ||
|
||
private handleErrorCases(url: string, statusCode: number): number { | ||
if (statusCode === 401 || statusCode === 403 || statusCode === 404) { | ||
return this.configurationError(url, statusCode); | ||
} else if ( | ||
statusCode === 429 || | ||
statusCode === 500 || | ||
statusCode === 502 || | ||
statusCode === 503 || | ||
statusCode === 504 | ||
) { | ||
return this.recoverableError(url, statusCode); | ||
} else { | ||
const error = new Error(`Response was not statusCode 2XX, but was ${statusCode}`); | ||
this.emit(UnleashEvents.Error, error); | ||
return this.refreshInterval; | ||
} | ||
} | ||
|
||
async fetch(): Promise<void> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed. Something like what we did for metrics. I'll have a look. |
||
if (this.stopped || !(this.refreshInterval > 0)) { | ||
return; | ||
} | ||
|
||
let nextFetch = this.refreshInterval; | ||
try { | ||
let mergedTags; | ||
|
@@ -257,7 +340,6 @@ Message: ${err.message}`, | |
const headers = this.customHeadersFunction | ||
? await this.customHeadersFunction() | ||
: this.headers; | ||
|
||
const res = await get({ | ||
url, | ||
etag: this.etag, | ||
|
@@ -268,14 +350,11 @@ Message: ${err.message}`, | |
httpOptions: this.httpOptions, | ||
supportedSpecVersion: SUPPORTED_SPEC_VERSION, | ||
}); | ||
|
||
if (res.status === 304) { | ||
// No new data | ||
this.emit(UnleashEvents.Unchanged); | ||
} else if (!res.ok) { | ||
const error = new Error(`Response was not statusCode 2XX, but was ${res.status}`); | ||
this.emit(UnleashEvents.Error, error); | ||
} else { | ||
} else if (res.ok) { | ||
nextFetch = this.countSuccess(); | ||
try { | ||
const data: ClientFeaturesResponse = await res.json(); | ||
if (res.headers.get('etag') !== null) { | ||
|
@@ -287,6 +366,8 @@ Message: ${err.message}`, | |
} catch (err) { | ||
this.emit(UnleashEvents.Error, err); | ||
} | ||
} else { | ||
nextFetch = this.handleErrorCases(url, res.status); | ||
} | ||
} catch (err) { | ||
const e = err as { code: string }; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typescript compiler stated that this was no longer necessary