From c4a20580ec663862deeaae3c247341d6ea0ab9d2 Mon Sep 17 00:00:00 2001 From: Niek Candaele Date: Sun, 20 Oct 2024 16:04:18 +0200 Subject: [PATCH 1/2] chore: parse axios errors always. Axios throws huge error objects which makes logging very verbose and it often includes sensitive data in the logs --- packages/app-api/src/lib/steamApi.ts | 5 ++- packages/app-api/src/service/StatsService.ts | 6 +-- packages/lib-apiclient/src/lib/baseAxios.ts | 42 +++++++++++++++++++ packages/lib-apiclient/src/lib/baseClient.ts | 5 ++- packages/lib-apiclient/src/main.ts | 1 + packages/lib-auth/src/lib/oryAxiosClient.ts | 5 ++- .../src/gameservers/7d2d/index.ts | 17 ++++---- .../src/gameservers/7d2d/sdtdAPIClient.ts | 31 ++------------ 8 files changed, 68 insertions(+), 44 deletions(-) create mode 100644 packages/lib-apiclient/src/lib/baseAxios.ts diff --git a/packages/app-api/src/lib/steamApi.ts b/packages/app-api/src/lib/steamApi.ts index 5f3adc1dd2..3262f88a28 100644 --- a/packages/app-api/src/lib/steamApi.ts +++ b/packages/app-api/src/lib/steamApi.ts @@ -1,6 +1,7 @@ -import axios, { AxiosError, AxiosInstance } from 'axios'; +import { AxiosError, AxiosInstance } from 'axios'; import { config } from '../config.js'; import { addCounterToAxios, errors, logger } from '@takaro/util'; +import { createAxios } from '@takaro/apiclient'; import { Redis } from '@takaro/db'; import ms from 'ms'; @@ -45,7 +46,7 @@ class SteamApi { get client() { if (!this._client) { - this._client = axios.create({ + this._client = createAxios({ baseURL: 'https://api.steampowered.com', timeout: 10000, }); diff --git a/packages/app-api/src/service/StatsService.ts b/packages/app-api/src/service/StatsService.ts index 36451b6cab..8b9732fb2d 100644 --- a/packages/app-api/src/service/StatsService.ts +++ b/packages/app-api/src/service/StatsService.ts @@ -2,8 +2,8 @@ import { TakaroService } from './Base.js'; import { TakaroDTO, errors, traceableClass } from '@takaro/util'; import { TakaroModel } from '@takaro/db'; +import { createAxios } from '@takaro/apiclient'; import { ITakaroRepo, PaginatedOutput } from '../db/base.js'; -import { Axios } from 'axios'; import { IsObject } from 'class-validator'; import { config } from '../config.js'; import { CountryStatsInputDTO, EventsCountInputDTO } from '../controllers/StatsController.js'; @@ -16,7 +16,7 @@ export class StatsOutputDTO extends TakaroDTO { @traceableClass('service:stats') export class StatsService extends TakaroService, TakaroDTO, TakaroDTO> { - private promClient = new Axios({ + private promClient = createAxios({ baseURL: config.get('metrics.prometheusUrl'), }); get repo(): ITakaroRepo, TakaroDTO, TakaroDTO> { @@ -60,7 +60,7 @@ export class StatsService extends TakaroService, Ta }, }); - const parsed = JSON.parse(response.data); + const parsed = response.data; if (parsed.status !== 'success') throw new errors.InternalServerError(); if (parsed.data.result.length === 0) return []; diff --git a/packages/lib-apiclient/src/lib/baseAxios.ts b/packages/lib-apiclient/src/lib/baseAxios.ts new file mode 100644 index 0000000000..b53c478526 --- /dev/null +++ b/packages/lib-apiclient/src/lib/baseAxios.ts @@ -0,0 +1,42 @@ +import axios, { AxiosError, CreateAxiosDefaults } from 'axios'; + +/** + * Creates an opinionated axios instance with defaults + * Includes error handling (Axios by default throws a HUGE error) + * @param opts CreateAxiosDefaults - Configuration for the axios instance + */ +export function createAxios(opts: CreateAxiosDefaults) { + const axiosInstance = axios.create(opts); + + // Strip down the error to a more manageable size + axiosInstance.interceptors.response.use( + (response) => response, + (error: AxiosError) => { + if (error.response) { + // The request was made and the server responded with a status code + // that falls out of the range of 2xx + const { status, statusText, headers, data } = error.response; + return Promise.reject({ + status, + statusText, + headers, + data, + }); + } else if (error.request) { + // The request was made but no response was received + // `error.request` is an instance of XMLHttpRequest in the browser and an instance of + // http.ClientRequest in node.js + return Promise.reject({ + message: 'No response received', + }); + } else { + // Something happened in setting up the request that triggered an Error + return Promise.reject({ + message: error.message, + }); + } + }, + ); + + return axiosInstance; +} diff --git a/packages/lib-apiclient/src/lib/baseClient.ts b/packages/lib-apiclient/src/lib/baseClient.ts index 02bfcb6e97..e7c2595c8c 100644 --- a/packages/lib-apiclient/src/lib/baseClient.ts +++ b/packages/lib-apiclient/src/lib/baseClient.ts @@ -1,5 +1,6 @@ import { MetaApi } from '../generated/api.js'; -import axios, { AxiosError, AxiosInstance, AxiosRequestConfig } from 'axios'; +import { AxiosError, AxiosInstance, AxiosRequestConfig } from 'axios'; +import { createAxios } from './baseAxios.js'; export interface IBaseApiClientConfig { url: string; @@ -31,7 +32,7 @@ export class BaseApiClient { }, withCredentials: true, }; - this.axios = this.addLoggers(axios.create(axiosConfig)); + this.axios = this.addLoggers(createAxios(axiosConfig)); if (this.config.log) this.log = this.config.log; } diff --git a/packages/lib-apiclient/src/main.ts b/packages/lib-apiclient/src/main.ts index f37b3f1021..ebf861545d 100644 --- a/packages/lib-apiclient/src/main.ts +++ b/packages/lib-apiclient/src/main.ts @@ -4,6 +4,7 @@ export { AxiosResponse } from 'axios'; export { isAxiosError } from 'axios'; export { AdminClient } from './lib/adminClient.js'; export { Client } from './lib/client.js'; +export { createAxios } from './lib/baseAxios.js'; export type ITakaroAPIAxiosResponse = AxiosResponse; export * from './generated/api.js'; diff --git a/packages/lib-auth/src/lib/oryAxiosClient.ts b/packages/lib-auth/src/lib/oryAxiosClient.ts index 86fd9a7bca..839d5569c2 100644 --- a/packages/lib-auth/src/lib/oryAxiosClient.ts +++ b/packages/lib-auth/src/lib/oryAxiosClient.ts @@ -1,10 +1,11 @@ -import axios, { AxiosError } from 'axios'; +import { AxiosError } from 'axios'; import { addCounterToAxios, errors, logger } from '@takaro/util'; +import { createAxios } from '@takaro/apiclient'; const log = logger('ory:http'); export function createAxiosClient(baseURL: string) { - const client = axios.create({ + const client = createAxios({ baseURL, headers: { 'Content-Type': 'application/json', diff --git a/packages/lib-gameserver/src/gameservers/7d2d/index.ts b/packages/lib-gameserver/src/gameservers/7d2d/index.ts index d9ae5aca31..05a973a6a6 100644 --- a/packages/lib-gameserver/src/gameservers/7d2d/index.ts +++ b/packages/lib-gameserver/src/gameservers/7d2d/index.ts @@ -122,17 +122,18 @@ export class SevenDaysToDie implements IGameServer { let reason = 'Unexpected error, this might be a bug'; this.logger.warn('Reachability test requests failed', error); - if (error instanceof Object && 'details' in error) { + if (error instanceof Object && !('status' in error)) { reason = 'Did not receive a response, please check that the server is running, the IP/port is correct and that it is not firewalled'; - if (error.details instanceof Object) { - if ('status' in error.details) { - if (error.details.status === 403 || error.details.status === 401) { - reason = 'Unauthorized, please check that the admin user and token are correct'; - } - } + } + + if (error instanceof Object && 'status' in error) { + if (error.status === 403 || error.status === 401) { + reason = 'Unauthorized, please check that the admin user and token are correct'; } - } else if (error instanceof Object && 'message' in error && error.message === 'Request timed out') { + } + + if (error instanceof Object && 'message' in error && error.message === 'Request timed out') { reason = 'Request timed out, the server did not respond in the allocated time'; } diff --git a/packages/lib-gameserver/src/gameservers/7d2d/sdtdAPIClient.ts b/packages/lib-gameserver/src/gameservers/7d2d/sdtdAPIClient.ts index ac7d6558df..da04646494 100644 --- a/packages/lib-gameserver/src/gameservers/7d2d/sdtdAPIClient.ts +++ b/packages/lib-gameserver/src/gameservers/7d2d/sdtdAPIClient.ts @@ -1,4 +1,4 @@ -import axios, { Axios, AxiosResponse } from 'axios'; +import { Axios, AxiosResponse } from 'axios'; import { SdtdConnectionInfo } from './connectionInfo.js'; import { CommandResponse, @@ -7,13 +7,14 @@ import { PlayerLocation, StatsResponse, } from './apiResponses.js'; -import { addCounterToAxios, errors } from '@takaro/util'; +import { addCounterToAxios } from '@takaro/util'; +import { createAxios } from '@takaro/apiclient'; export class SdtdApiClient { private client: Axios; constructor(private config: SdtdConnectionInfo) { - this.client = axios.create({ + this.client = createAxios({ baseURL: this.url, }); @@ -28,30 +29,6 @@ export class SdtdApiClient { return req; }); - - this.client.interceptors.response.use( - function (response) { - return response; - }, - function (error) { - // Any status codes that falls outside the range of 2xx cause this function to trigger - if (error.response) { - const simplifiedError = new errors.BadRequestError('Axios error', { - extra: 'A request to the 7D2D server failed', - status: error.response.status, - statusText: error.response.statusText, - url: error.config.url, - }); - return Promise.reject(simplifiedError); - } - const simplifiedError = new errors.BadRequestError('Axios error', { - extra: 'A request to the 7D2D server failed', - message: error.message, - url: error.config.url, - }); - return Promise.reject(simplifiedError); - }, - ); } private get url() { From 0184efc4d05d1cf50448bdab4351658da9c0f943 Mon Sep 17 00:00:00 2001 From: Niek Candaele Date: Thu, 24 Oct 2024 21:48:44 +0200 Subject: [PATCH 2/2] fix: better axios logging --- packages/app-api/src/lib/steamApi.ts | 62 ++--------- packages/app-api/src/service/StatsService.ts | 11 +- packages/lib-apiclient/src/lib/baseAxios.ts | 104 +++++++++++++----- packages/lib-apiclient/src/lib/baseClient.ts | 57 +--------- packages/lib-apiclient/src/main.ts | 2 +- packages/lib-auth/src/lib/oryAxiosClient.ts | 55 ++------- .../src/gameservers/7d2d/sdtdAPIClient.ts | 12 +- 7 files changed, 116 insertions(+), 187 deletions(-) diff --git a/packages/app-api/src/lib/steamApi.ts b/packages/app-api/src/lib/steamApi.ts index 3262f88a28..311be7eddb 100644 --- a/packages/app-api/src/lib/steamApi.ts +++ b/packages/app-api/src/lib/steamApi.ts @@ -1,4 +1,4 @@ -import { AxiosError, AxiosInstance } from 'axios'; +import { AxiosInstance } from 'axios'; import { config } from '../config.js'; import { addCounterToAxios, errors, logger } from '@takaro/util'; import { createAxios } from '@takaro/apiclient'; @@ -46,10 +46,13 @@ class SteamApi { get client() { if (!this._client) { - this._client = createAxios({ - baseURL: 'https://api.steampowered.com', - timeout: 10000, - }); + this._client = createAxios( + { + baseURL: 'https://api.steampowered.com', + timeout: 10000, + }, + { logger: this.log }, + ); addCounterToAxios(this._client, { name: 'steam_api_requests_total', @@ -78,55 +81,6 @@ class SteamApi { config.params.key = this.apiKey; return config; }); - - this._client.interceptors.request.use((request) => { - this.log.debug(`➡️ ${request.method?.toUpperCase()} ${request.url}`, { - method: request.method, - url: request.url, - }); - return request; - }); - - this._client.interceptors.response.use( - (response) => { - this.log.debug( - `⬅️ ${response.request.method?.toUpperCase()} ${response.request.path} ${response.status} ${ - response.statusText - }`, - { - status: response.status, - statusText: response.statusText, - method: response.request.method, - url: response.request.url, - }, - ); - - return response; - }, - async (error: AxiosError) => { - let details = {}; - - if (error.response?.data) { - const data = error.response.data as Record; - details = JSON.stringify(data.meta); - } - - if (error.response?.status === 429) { - await this.setRateLimited(); - } - - this.log.error('☠️ Request errored', { - traceId: error.response?.headers['x-trace-id'], - details, - status: error.response?.status, - statusText: error.response?.statusText, - method: error.config?.method, - url: error.config?.url, - response: error.response?.data, - }); - return Promise.reject(error); - }, - ); } return this._client; } diff --git a/packages/app-api/src/service/StatsService.ts b/packages/app-api/src/service/StatsService.ts index 8b9732fb2d..22f3a8b8f8 100644 --- a/packages/app-api/src/service/StatsService.ts +++ b/packages/app-api/src/service/StatsService.ts @@ -16,9 +16,14 @@ export class StatsOutputDTO extends TakaroDTO { @traceableClass('service:stats') export class StatsService extends TakaroService, TakaroDTO, TakaroDTO> { - private promClient = createAxios({ - baseURL: config.get('metrics.prometheusUrl'), - }); + private promClient = createAxios( + { + baseURL: config.get('metrics.prometheusUrl'), + }, + { + logger: this.log, + }, + ); get repo(): ITakaroRepo, TakaroDTO, TakaroDTO> { // Dummy since we're not talking to our DB here return {} as ITakaroRepo, TakaroDTO, TakaroDTO>; diff --git a/packages/lib-apiclient/src/lib/baseAxios.ts b/packages/lib-apiclient/src/lib/baseAxios.ts index b53c478526..68c867e5b8 100644 --- a/packages/lib-apiclient/src/lib/baseAxios.ts +++ b/packages/lib-apiclient/src/lib/baseAxios.ts @@ -1,42 +1,92 @@ import axios, { AxiosError, CreateAxiosDefaults } from 'axios'; +interface ILog { + error: (message: string, meta: Record) => void; + debug: (message: string, meta: Record) => void; + info: (message: string, meta: Record) => void; + warn: (message: string, meta: Record) => void; +} + +interface ITakaroAxiosOptions { + logger?: ILog; +} + +export class CleanAxiosError extends AxiosError { + constructor(axiosError: AxiosError) { + super(axiosError.message, axiosError.code, axiosError.config, axiosError.request, axiosError.response); + + if (axiosError.response) { + const { status, statusText } = axiosError.response; + this.message = `Request failed with status ${status} ${statusText}`; + } else if (axiosError.request) { + this.message = 'No response received'; + } else { + this.message = axiosError.message; + } + + // Strip down the error to a more manageable size when printing it + // By default Axios throws a HUGE error + this.toJSON = () => ({ + message: this.message, + name: this.name, + method: axiosError.config?.method, + url: axiosError.config?.url, + status: axiosError.response?.status, + data: axiosError.response?.data, + requestTraceId: axiosError.response?.headers['x-trace-id'], + }); + } +} + /** * Creates an opinionated axios instance with defaults - * Includes error handling (Axios by default throws a HUGE error) * @param opts CreateAxiosDefaults - Configuration for the axios instance */ -export function createAxios(opts: CreateAxiosDefaults) { +export function createAxios(opts: CreateAxiosDefaults, { logger }: ITakaroAxiosOptions = {}) { const axiosInstance = axios.create(opts); - // Strip down the error to a more manageable size axiosInstance.interceptors.response.use( - (response) => response, + function (response) { + return response; + }, (error: AxiosError) => { - if (error.response) { - // The request was made and the server responded with a status code - // that falls out of the range of 2xx - const { status, statusText, headers, data } = error.response; - return Promise.reject({ - status, - statusText, - headers, - data, - }); - } else if (error.request) { - // The request was made but no response was received - // `error.request` is an instance of XMLHttpRequest in the browser and an instance of - // http.ClientRequest in node.js - return Promise.reject({ - message: 'No response received', - }); - } else { - // Something happened in setting up the request that triggered an Error - return Promise.reject({ - message: error.message, - }); - } + return Promise.reject(new CleanAxiosError(error)); }, ); + // Add detailed logging + if (logger) { + axiosInstance.interceptors.request.use((request) => { + logger.info(`➡️ ${request.method?.toUpperCase()} ${request.url}`, { + method: request.method, + url: request.url, + requestTraceId: request.headers['x-trace-id'], + }); + return request; + }); + + axiosInstance.interceptors.response.use( + (response) => { + logger.info( + `⬅️ ${response.request.method?.toUpperCase()} ${response.request.path} ${response.status} ${ + response.statusText + }`, + { + status: response.status, + method: response.request.method, + url: response.request.url, + requestTraceId: response.headers['x-trace-id'], + }, + ); + + return response; + }, + (error: AxiosError) => { + logger.error('☠️ Request errored', error.toJSON()); + return Promise.reject(error); + }, + ); + } + return axiosInstance; } diff --git a/packages/lib-apiclient/src/lib/baseClient.ts b/packages/lib-apiclient/src/lib/baseClient.ts index e7c2595c8c..39ad6c58fa 100644 --- a/packages/lib-apiclient/src/lib/baseClient.ts +++ b/packages/lib-apiclient/src/lib/baseClient.ts @@ -1,5 +1,5 @@ import { MetaApi } from '../generated/api.js'; -import { AxiosError, AxiosInstance, AxiosRequestConfig } from 'axios'; +import { AxiosInstance, AxiosRequestConfig } from 'axios'; import { createAxios } from './baseAxios.js'; export interface IBaseApiClientConfig { @@ -32,9 +32,9 @@ export class BaseApiClient { }, withCredentials: true, }; - this.axios = this.addLoggers(createAxios(axiosConfig)); if (this.config.log) this.log = this.config.log; + this.axios = createAxios(axiosConfig, { logger: this.log }); } setHeader(key: string, value: string) { @@ -49,59 +49,6 @@ export class BaseApiClient { return mime === 'application/json'; } - private addLoggers(axios: AxiosInstance): AxiosInstance { - if (this.config.log === false) { - return axios; - } - - axios.interceptors.request.use((request) => { - this.log.info(`➡️ ${request.method?.toUpperCase()} ${request.url}`, { - method: request.method, - url: request.url, - }); - return request; - }); - - axios.interceptors.response.use( - (response) => { - this.log.info( - `⬅️ ${response.request.method?.toUpperCase()} ${response.request.path} ${response.status} ${ - response.statusText - }`, - { - status: response.status, - statusText: response.statusText, - method: response.request.method, - url: response.request.url, - }, - ); - - return response; - }, - (error: AxiosError) => { - let details = {}; - - if (error.response?.data) { - const data = error.response.data as Record; - details = JSON.stringify(data.meta); - } - - this.log.error('☠️ Request errored', { - traceId: error.response?.headers['x-trace-id'], - details, - status: error.response?.status, - statusText: error.response?.statusText, - method: error.config?.method, - url: error.config?.url, - response: error.response?.data, - }); - return Promise.reject(error); - }, - ); - - return axios; - } - /** * Wait until the API reports that it is healthy * @param timeout in milliseconds diff --git a/packages/lib-apiclient/src/main.ts b/packages/lib-apiclient/src/main.ts index ebf861545d..3e3e91ea2e 100644 --- a/packages/lib-apiclient/src/main.ts +++ b/packages/lib-apiclient/src/main.ts @@ -4,7 +4,7 @@ export { AxiosResponse } from 'axios'; export { isAxiosError } from 'axios'; export { AdminClient } from './lib/adminClient.js'; export { Client } from './lib/client.js'; -export { createAxios } from './lib/baseAxios.js'; +export { createAxios, CleanAxiosError } from './lib/baseAxios.js'; export type ITakaroAPIAxiosResponse = AxiosResponse; export * from './generated/api.js'; diff --git a/packages/lib-auth/src/lib/oryAxiosClient.ts b/packages/lib-auth/src/lib/oryAxiosClient.ts index 839d5569c2..4465b1fdc9 100644 --- a/packages/lib-auth/src/lib/oryAxiosClient.ts +++ b/packages/lib-auth/src/lib/oryAxiosClient.ts @@ -2,65 +2,34 @@ import { AxiosError } from 'axios'; import { addCounterToAxios, errors, logger } from '@takaro/util'; import { createAxios } from '@takaro/apiclient'; -const log = logger('ory:http'); - export function createAxiosClient(baseURL: string) { - const client = createAxios({ - baseURL, - headers: { - 'Content-Type': 'application/json', - 'User-Agent': 'Takaro-Agent', + const log = logger('ory:http'); + const client = createAxios( + { + baseURL, + headers: { + 'Content-Type': 'application/json', + 'User-Agent': 'Takaro-Agent', + }, }, - }); + { + logger: log, + }, + ); addCounterToAxios(client, { name: 'ory_api_requests_total', help: 'Total number of requests to the Ory API', }); - client.interceptors.request.use((request) => { - log.silly(`➡️ ${request.method?.toUpperCase()} ${request.url}`, { - method: request.method, - url: request.url, - }); - return request; - }); - client.interceptors.response.use( (response) => { - log.silly( - `⬅️ ${response.request.method?.toUpperCase()} ${response.request.path} ${response.status} ${ - response.statusText - }`, - { - status: response.status, - method: response.request.method, - url: response.request.url, - }, - ); - return response; }, (error: AxiosError) => { - let details = {}; - - if (error.response?.data) { - const data = error.response.data as Record; - details = JSON.stringify(data.error_description); - } - - log.error(`☠️ Request errored: [${error.response?.status}] ${details}`, { - status: error.response?.status, - statusText: error.response?.statusText, - method: error.config?.method, - url: error.config?.url, - response: error.response?.data, - }); - if (error.response?.status === 409) { return Promise.reject(new errors.ConflictError('User with this identifier already exists')); } - return Promise.reject(error); }, ); diff --git a/packages/lib-gameserver/src/gameservers/7d2d/sdtdAPIClient.ts b/packages/lib-gameserver/src/gameservers/7d2d/sdtdAPIClient.ts index da04646494..1e205882e9 100644 --- a/packages/lib-gameserver/src/gameservers/7d2d/sdtdAPIClient.ts +++ b/packages/lib-gameserver/src/gameservers/7d2d/sdtdAPIClient.ts @@ -7,16 +7,20 @@ import { PlayerLocation, StatsResponse, } from './apiResponses.js'; -import { addCounterToAxios } from '@takaro/util'; +import { addCounterToAxios, logger } from '@takaro/util'; import { createAxios } from '@takaro/apiclient'; export class SdtdApiClient { private client: Axios; + private log = logger('7d2d:api'); constructor(private config: SdtdConnectionInfo) { - this.client = createAxios({ - baseURL: this.url, - }); + this.client = createAxios( + { + baseURL: this.url, + }, + { logger: this.log }, + ); addCounterToAxios(this.client, { name: 'sdtd_api_requests_total',