Skip to content

Commit

Permalink
fix(http): catch uncaught get errors, return best effort error mess…
Browse files Browse the repository at this point in the history
…age PE-7598
  • Loading branch information
fedellen committed Feb 7, 2025
1 parent 34ba3f7 commit c884edc
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 39 deletions.
64 changes: 33 additions & 31 deletions src/common/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { AxiosInstance } from 'axios';
import { AxiosError, AxiosInstance, AxiosResponse, CanceledError } from 'axios';
import { IAxiosRetryConfig } from 'axios-retry';
import { Buffer } from 'node:buffer';
import { Readable } from 'stream';
Expand Down Expand Up @@ -60,6 +60,7 @@ export class TurboHTTPService implements TurboHTTPServiceInterface {
logger: this.logger,
});
}

async get<T>({
endpoint,
signal,
Expand All @@ -71,20 +72,10 @@ export class TurboHTTPService implements TurboHTTPServiceInterface {
allowedStatuses?: number[];
headers?: Partial<TurboSignedRequestHeaders> & Record<string, string>;
}): Promise<T> {
const { status, statusText, data } = await this.axios.get<T>(endpoint, {
headers,
signal,
});

if (!allowedStatuses.includes(status)) {
throw new FailedRequestError(
status,
// Return error message from server if available
typeof data === 'string' ? data : statusText,
);
}

return data;
return this.tryRequest<T>(
() => this.axios.get<T>(endpoint, { headers, signal }),
allowedStatuses,
);
}

async post<T>({
Expand All @@ -100,23 +91,34 @@ export class TurboHTTPService implements TurboHTTPServiceInterface {
headers?: Partial<TurboSignedRequestHeaders> & Record<string, string>;
data: Readable | Buffer | ReadableStream;
}): Promise<T> {
const {
status,
statusText,
data: response,
} = await this.axios.post<T>(endpoint, data, {
headers,
signal,
});
return this.tryRequest(
() => this.axios.post<T>(endpoint, data, { headers, signal }),
allowedStatuses,
);
}

if (!allowedStatuses.includes(status)) {
throw new FailedRequestError(
status,
// Return error message from server if available
typeof response === 'string' ? response : statusText,
);
private async tryRequest<T>(
request: () => Promise<AxiosResponse<T, unknown>>,
allowedStatuses: number[],
): Promise<T> {
try {
const { status, data, statusText } = await request();
if (!allowedStatuses.includes(status)) {
throw new FailedRequestError(
// Return error message from server if available
typeof data === 'string' ? data : statusText,
status,
);
}
return data;
} catch (error) {
if (error instanceof CanceledError) {
throw error;
}
if (error instanceof AxiosError) {
throw new FailedRequestError(error.code ?? error.message, error.status);
}
throw error;
}

return response;
}
}
10 changes: 6 additions & 4 deletions src/common/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ export abstract class TurboAuthenticatedBaseUploadService
const retryDelay =
this.retryConfig.retryDelay ??
((retryNumber: number) => retryNumber * 1000);
let lastError = 'Unknown Error'; // Store the last error for throwing
let lastStatusCode: number | undefined; // Store the last status code for throwing
let lastError: string | undefined = undefined; // Store the last error for throwing
let lastStatusCode: number | undefined = undefined; // Store the last status code for throwing

while (retries < maxRetries) {
if (signal?.aborted) {
Expand Down Expand Up @@ -196,8 +196,10 @@ export abstract class TurboAuthenticatedBaseUploadService

// After all retries, throw the last error for catching
throw new FailedRequestError(
lastStatusCode ?? 500,
`Failed to upload file after ${maxRetries} attempts: ${lastError}`,
`Failed to upload file after ${maxRetries} attempts${
lastError !== undefined && lastError.length > 0 ? `: ${lastError}` : ''
}`,
lastStatusCode,
);
}

Expand Down
3 changes: 1 addition & 2 deletions src/utils/axiosClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ export const createAxiosInstance = ({
validateStatus: () => true, // don't throw on non-200 status codes
});

// eslint-disable-next-line
if (retryConfig.retries && retryConfig.retries > 0) {
if (retryConfig.retries !== undefined && retryConfig.retries > 0) {
axiosRetry(axiosInstance, retryConfig);
}

Expand Down
6 changes: 4 additions & 2 deletions src/utils/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ export class UnauthenticatedRequestError extends BaseError {
}

export class FailedRequestError extends BaseError {
constructor(status: number, message: string) {
super(`Failed request: ${status}: ${message}`);
constructor(message: string, status?: number) {
super(
`Failed request:${status !== undefined ? ` ${status}:` : ''} ${message}`,
);
}
}

Expand Down

0 comments on commit c884edc

Please sign in to comment.