Skip to content

Use axios for fetching challenges #12

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

Merged
merged 1 commit into from
Jul 10, 2025
Merged

Use axios for fetching challenges #12

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
68 changes: 34 additions & 34 deletions src/mcp/tools/challenges/queryChallenges.tool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,66 +50,66 @@ export class QueryChallengesTool {
accessToken,
);

if (!challenges.ok) {
if (challenges.status < 200 || challenges.status >= 300) {
Copy link

Choose a reason for hiding this comment

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

low
readability
The condition challenges.status < 200 || challenges.status >= 300 is technically correct for checking non-success HTTP status codes, but consider using !challenges.status.toString().startsWith('2') for improved readability and to clearly convey the intent of checking for non-2xx status codes.

this.logger.error(
`Failed to fetch challenges from Topcoder API: ${challenges.statusText}`,
);
try {
this.logger.error(await challenges.json());
this.logger.error(challenges.data);
Copy link

Choose a reason for hiding this comment

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

medium
correctness
The change from await challenges.json() to challenges.data assumes that challenges.data is always available and correctly formatted. Ensure that challenges.data is reliably structured as expected, or add error handling for cases where it might not be.

} catch (e) {
this.logger.error('Failed to log challenge error');
}

// Return an error response if the API call fails
return {
content: [
{
type: 'text',
text: `Error fetching challenges: ${challenges.statusText}`,
},
{
type: 'text',
text: `Error fetching challenges: ${challenges.statusText}`,
},
],
isError: true,
};
}

// Parse the response as JSON
const challengesData = await challenges.json();
// Axios response: data is already parsed, headers are plain object
const challengesData = challenges.data;

return {
content: [
{
type: 'text',
text: JSON.stringify({
page: Number(challenges.headers.get('x-page')) || 1,
pageSize:
Number(challenges.headers.get('x-per-page')) ||
challengesData.length ||
0,
total:
Number(challenges.headers.get('x-total')) ||
challengesData.length ||
0,
nextPage: challenges.headers.get('x-next-page')
? Number(challenges.headers.get('x-next-page'))
: null,
data: challengesData,
}),
},
],
structuredContent: {
page: Number(challenges.headers.get('x-page')) || 1,
type: 'text',
text: JSON.stringify({
page: Number(challenges.headers['x-page']) || 1,
Copy link

Choose a reason for hiding this comment

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

medium
correctness
Consider checking if challenges.headers['x-page'] is a valid number before using Number() to avoid potential NaN values.

pageSize:
Number(challenges.headers.get('x-per-page')) ||
challengesData.length ||
Number(challenges.headers['x-per-page']) ||
(Array.isArray(challengesData) ? challengesData.length : 0) ||
Copy link

Choose a reason for hiding this comment

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

medium
correctness
Ensure challenges.headers['x-per-page'] is a valid number before conversion to prevent NaN results, which could affect pagination logic.

0,
total:
Number(challenges.headers.get('x-total')) ||
challengesData.length ||
Number(challenges.headers['x-total']) ||
Copy link

Choose a reason for hiding this comment

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

medium
correctness
Validate challenges.headers['x-total'] to ensure it is a number before conversion to avoid potential NaN issues.

(Array.isArray(challengesData) ? challengesData.length : 0) ||
0,
nextPage: challenges.headers.get('x-next-page')
? Number(challenges.headers.get('x-next-page'))
nextPage: challenges.headers['x-next-page']
Copy link

Choose a reason for hiding this comment

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

medium
correctness
Consider checking if challenges.headers['x-next-page'] is a valid number before using Number() to avoid potential NaN values.

? Number(challenges.headers['x-next-page'])
: null,
data: challengesData,
}),
},
],
structuredContent: {
page: Number(challenges.headers['x-page']) || 1,
pageSize:
Number(challenges.headers['x-per-page']) ||
(Array.isArray(challengesData) ? challengesData.length : 0) ||
0,
total:
Number(challenges.headers['x-total']) ||
(Array.isArray(challengesData) ? challengesData.length : 0) ||
0,
nextPage: challenges.headers['x-next-page']
? Number(challenges.headers['x-next-page'])
: null,
data: challengesData,
},
};
} catch (error) {
Expand Down
7 changes: 3 additions & 4 deletions src/shared/topcoder/challenges.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ENV_CONFIG } from 'src/config';
import { Logger } from 'src/shared/global';
import { QUERY_CHALLENGES_TOOL_PARAMETERS } from 'src/mcp/tools/challenges/queryChallenges.parameters';
import { z } from 'zod';
import axios from 'axios';

const { TOPCODER_API_BASE_URL } = ENV_CONFIG;

Expand Down Expand Up @@ -40,10 +41,8 @@ export class TopcoderChallengesService {
);

try {
return await fetch(stringUrl, {
method: 'GET',
headers,
});
const response = await axios.get(stringUrl, { headers });
return response;
Copy link

Choose a reason for hiding this comment

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

medium
design
Consider returning response.data instead of the entire response object. Axios responses include metadata that may not be necessary for the caller, and returning only the data improves encapsulation and clarity of the service's API.

} catch (error) {
this.logger.error(`Error fetching challenges: ${JSON.stringify(error)}`, error);
throw error;
Expand Down