Skip to content

Commit 2b19d03

Browse files
csumana19Copilot
andcommitted
Address review: only set rateLimit when headers present, handle collections, add 429 and no-header tests
- Only assign target.rateLimit when at least one rate limit header exists - Attach rateLimit to response.result.value for collection responses - Add unit test for 429 error path with rateLimit on error object - Add unit test verifying rateLimit is undefined when no headers present - Remove redundant IRestResponse cast Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent bc7f4c9 commit 2b19d03

3 files changed

Lines changed: 73 additions & 3 deletions

File tree

api/AdoHttpClientBases.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ export class AdoRestClient extends rm.RestClient {
3434
const response = await super.processResponse(res, options);
3535
if (response && response.result && typeof response.result === 'object') {
3636
extractRateLimitHeaders(headers, response.result);
37+
// For collection responses (e.g., { count, value: [...] }), also attach to the array
38+
if (Array.isArray((response.result as any).value)) {
39+
extractRateLimitHeaders(headers, (response.result as any).value);
40+
}
3741
}
3842
return response as rm.IRestResponse<T>;
3943
} catch (err: any) {

api/RateLimitUtils.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,24 +14,34 @@ export function extractRateLimitHeaders(headers: any, target: any): void {
1414
}
1515

1616
const rateLimit: VsoBaseInterfaces.RateLimit = {};
17+
let hasRateLimitHeader = false;
1718

1819
if (headers['x-ratelimit-resource']) {
1920
rateLimit.resource = headers['x-ratelimit-resource'];
21+
hasRateLimitHeader = true;
2022
}
2123
if (headers['x-ratelimit-delay']) {
2224
rateLimit.delay = parseFloat(headers['x-ratelimit-delay']);
25+
hasRateLimitHeader = true;
2326
}
2427
if (headers['x-ratelimit-limit']) {
2528
rateLimit.limit = parseInt(headers['x-ratelimit-limit'], 10);
29+
hasRateLimitHeader = true;
2630
}
2731
if (headers['x-ratelimit-remaining']) {
2832
rateLimit.remaining = parseInt(headers['x-ratelimit-remaining'], 10);
33+
hasRateLimitHeader = true;
2934
}
3035
if (headers['x-ratelimit-reset']) {
3136
rateLimit.reset = parseInt(headers['x-ratelimit-reset'], 10);
37+
hasRateLimitHeader = true;
3238
}
3339
if (headers['retry-after']) {
3440
rateLimit.retryAfter = parseInt(headers['retry-after'], 10);
41+
hasRateLimitHeader = true;
42+
}
43+
44+
if (hasRateLimitHeader) {
45+
target.rateLimit = rateLimit;
3546
}
36-
target.rateLimit = rateLimit;
3747
}

test/units/tests.ts

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import WebApi = require('../../_build/WebApi');
77
import * as rm from 'typed-rest-client/RestClient';
88
import { ApiResourceLocation, RateLimit } from '../../_build/interfaces/common/VsoBaseInterfaces';
99
import semver = require('semver');
10-
import { IRestResponse } from 'typed-rest-client';
1110

1211
describe('VSOClient Units', function () {
1312
let rest: rm.RestClient;
@@ -341,7 +340,7 @@ describe('WebApi Units', function () {
341340
const myWebApi: WebApi.WebApi = new WebApi.WebApi('https://dev.azure.com', WebApi.getBasicHandler('user', 'password'));
342341

343342
// Act
344-
const response = (await myWebApi.rest.get<{ success: boolean, rateLimit?: RateLimit }>('https://dev.azure.com/test')) as IRestResponse<{ success: boolean, rateLimit?: RateLimit }>;
343+
const response = await myWebApi.rest.get<{ success: boolean, rateLimit?: RateLimit }>('https://dev.azure.com/test');
345344
// Assert
346345
assert(response.result?.success === true, 'Response body should indicate success');
347346
assert(response.result?.rateLimit, 'Response should have rateLimit property');
@@ -353,6 +352,63 @@ describe('WebApi Units', function () {
353352
assert.equal(response.result.rateLimit?.retryAfter, 1, 'Rate limit retryAfter should be 1');
354353
});
355354

355+
it('AdoRestClient attaches rate limit headers to error on 429', async () => {
356+
// Arrange
357+
const userAgent: string = `${nodeApiName}/${nodeApiVersion} (${osName} ${osVersion})`;
358+
359+
nock('https://dev.azure.com', {
360+
reqheaders: {
361+
'accept': 'application/json',
362+
'user-agent': userAgent,
363+
},
364+
})
365+
.defaultReplyHeaders({
366+
'x-ratelimit-resource': 'core',
367+
'x-ratelimit-delay': '5.0',
368+
'x-ratelimit-limit': '1000',
369+
'x-ratelimit-remaining': '0',
370+
'x-ratelimit-reset': '1625078400',
371+
'retry-after': '30',
372+
})
373+
.get('/blocked')
374+
.reply(429, { message: 'Too Many Requests' });
375+
376+
const myWebApi: WebApi.WebApi = new WebApi.WebApi('https://dev.azure.com', WebApi.getBasicHandler('user', 'password'));
377+
378+
// Act & Assert
379+
try {
380+
await myWebApi.rest.get<any>('https://dev.azure.com/blocked');
381+
assert.fail('Should have thrown on 429');
382+
} catch (err: any) {
383+
assert(err.rateLimit, 'Error should have rateLimit property');
384+
assert.equal(err.rateLimit.resource, 'core', 'Rate limit resource should be "core"');
385+
assert.equal(err.rateLimit.remaining, 0, 'Rate limit remaining should be 0');
386+
assert.equal(err.rateLimit.retryAfter, 30, 'Rate limit retryAfter should be 30');
387+
}
388+
});
389+
390+
it('AdoRestClient does not attach rateLimit when no rate limit headers present', async () => {
391+
// Arrange
392+
const userAgent: string = `${nodeApiName}/${nodeApiVersion} (${osName} ${osVersion})`;
393+
394+
nock('https://dev.azure.com', {
395+
reqheaders: {
396+
'accept': 'application/json',
397+
'user-agent': userAgent,
398+
},
399+
})
400+
.get('/no-ratelimit')
401+
.reply(200, { data: 'hello' });
402+
403+
const myWebApi: WebApi.WebApi = new WebApi.WebApi('https://dev.azure.com', WebApi.getBasicHandler('user', 'password'));
404+
405+
// Act
406+
const response = await myWebApi.rest.get<{ data: string, rateLimit?: RateLimit }>('https://dev.azure.com/no-ratelimit');
407+
// Assert
408+
assert(response.result?.data === 'hello', 'Response body should have data');
409+
assert.equal(response.result?.rateLimit, undefined, 'rateLimit should not be present when no rate limit headers');
410+
});
411+
356412
it('sets the user agent correctly when request settings are not specified', async () => {
357413
const myWebApi: WebApi.WebApi = new WebApi.WebApi('https://microsoft.com', WebApi.getBasicHandler('user', 'password'), undefined);
358414
const userAgent: string = `${nodeApiName}/${nodeApiVersion} (${osName} ${osVersion})`;

0 commit comments

Comments
 (0)