Use axios for fetching challenges #12
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -50,66 +50,66 @@ export class QueryChallengesTool { | |||
| accessToken, | |||
| ); | |||
|
|
|||
| if (!challenges.ok) { | |||
| if (challenges.status < 200 || challenges.status >= 300) { | |||
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
| page: Number(challenges.headers.get('x-page')) || 1, | ||
| type: 'text', | ||
| text: JSON.stringify({ | ||
| page: Number(challenges.headers['x-page']) || 1, |
There was a problem hiding this comment.
medium
correctness
Consider checking if challenges.headers['x-page'] is a valid number before using Number() to avoid potential NaN values.
| Number(challenges.headers.get('x-per-page')) || | ||
| challengesData.length || | ||
| Number(challenges.headers['x-per-page']) || | ||
| (Array.isArray(challengesData) ? challengesData.length : 0) || |
There was a problem hiding this comment.
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']) || |
There was a problem hiding this comment.
medium
correctness
Validate challenges.headers['x-total'] to ensure it is a number before conversion to avoid potential NaN issues.
| 0, | ||
| nextPage: challenges.headers.get('x-next-page') | ||
| ? Number(challenges.headers.get('x-next-page')) | ||
| nextPage: challenges.headers['x-next-page'] |
There was a problem hiding this comment.
medium
correctness
Consider checking if challenges.headers['x-next-page'] is a valid number before using Number() to avoid potential NaN values.
| headers, | ||
| }); | ||
| const response = await axios.get(stringUrl, { headers }); | ||
| return response; |
There was a problem hiding this comment.
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.
No description provided.