fix(sdk): cap concurrent UploadApplicationFile requests
https://sonarly.com/issue/39055?type=bug The UploadApplicationFile mutation is being throttled at 30 requests / 30s per app+workspace, causing app dev/deploy uploads to fail mid-run. This blocks application sync/publish flows for apps with enough files or bursty uploads. Fix: I implemented the user-impacting fix in the SDK upload path (`appDevOnce`) by replacing unbounded `Promise.all` file uploads with rate-safe batching: - Added `APP_DEV_UPLOAD_BATCH_SIZE = 25` - Added `APP_DEV_UPLOAD_BATCH_DELAY_MS = 30_000` - Uploads now run in batches of 25, then wait 30s before the next batch Why this fixes the incident: - Server enforces 30 tokens / 30s per `app-dev:${workspaceId}:${applicationIdentifier}` - SDK previously fired all uploads at once, exhausting tokens and failing mid-run - Batching to 25 per window avoids token bucket exhaustion and preserves headroom for other app-dev calls in the same flow (`createDevelopmentApplication`, `syncApplication`) that share the throttle key. Authored by Sonarly by autonomous analysis (run 44395).
This commit is contained in:
@@ -32,6 +32,9 @@ export type AppDevOnceResult = {
|
||||
applicationUniversalIdentifier: string;
|
||||
};
|
||||
|
||||
const APP_DEV_UPLOAD_BATCH_SIZE = 25;
|
||||
const APP_DEV_UPLOAD_BATCH_DELAY_MS = 30_000;
|
||||
|
||||
const innerAppDevOnce = async (
|
||||
options: AppDevOnceOptions,
|
||||
): Promise<CommandResult<AppDevOnceResult>> => {
|
||||
@@ -158,27 +161,46 @@ const innerAppDevOnce = async (
|
||||
});
|
||||
|
||||
const uploadErrors: string[] = [];
|
||||
const builtFileInfos = Array.from(buildResult.builtFileInfos.values());
|
||||
|
||||
const uploadPromises = Array.from(buildResult.builtFileInfos.values()).map(
|
||||
async (builtFileInfo) => {
|
||||
if (verbose) {
|
||||
onProgress?.(`Uploading ${builtFileInfo.builtPath}`);
|
||||
}
|
||||
for (
|
||||
let startIndex = 0;
|
||||
startIndex < builtFileInfos.length;
|
||||
startIndex += APP_DEV_UPLOAD_BATCH_SIZE
|
||||
) {
|
||||
const uploadBatch = builtFileInfos.slice(
|
||||
startIndex,
|
||||
startIndex + APP_DEV_UPLOAD_BATCH_SIZE,
|
||||
);
|
||||
|
||||
const result = await fileUploader.uploadFile({
|
||||
builtPath: builtFileInfo.builtPath,
|
||||
fileFolder: builtFileInfo.fileFolder,
|
||||
});
|
||||
await Promise.all(
|
||||
uploadBatch.map(async (builtFileInfo) => {
|
||||
if (verbose) {
|
||||
onProgress?.(`Uploading ${builtFileInfo.builtPath}`);
|
||||
}
|
||||
|
||||
if (!result.success) {
|
||||
uploadErrors.push(
|
||||
`Failed to upload ${builtFileInfo.builtPath}: ${serializeError(result.error)}`,
|
||||
);
|
||||
}
|
||||
},
|
||||
);
|
||||
const result = await fileUploader.uploadFile({
|
||||
builtPath: builtFileInfo.builtPath,
|
||||
fileFolder: builtFileInfo.fileFolder,
|
||||
});
|
||||
|
||||
await Promise.all(uploadPromises);
|
||||
if (!result.success) {
|
||||
uploadErrors.push(
|
||||
`Failed to upload ${builtFileInfo.builtPath}: ${serializeError(result.error)}`,
|
||||
);
|
||||
}
|
||||
}),
|
||||
);
|
||||
|
||||
const hasMoreBatches =
|
||||
startIndex + APP_DEV_UPLOAD_BATCH_SIZE < builtFileInfos.length;
|
||||
|
||||
if (hasMoreBatches) {
|
||||
await new Promise((resolve) =>
|
||||
setTimeout(resolve, APP_DEV_UPLOAD_BATCH_DELAY_MS),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (uploadErrors.length > 0) {
|
||||
return {
|
||||
|
||||
+34
@@ -0,0 +1,34 @@
|
||||
import { GraphQLError } from 'graphql';
|
||||
|
||||
import {
|
||||
ThrottlerException,
|
||||
ThrottlerExceptionCode,
|
||||
} from 'src/engine/core-modules/throttler/throttler.exception';
|
||||
import { shouldCaptureException } from 'src/engine/utils/global-exception-handler.util';
|
||||
|
||||
describe('shouldCaptureException', () => {
|
||||
it('should not capture graphql errors with status code under 500', () => {
|
||||
const graphQLError = new GraphQLError('Forbidden', {
|
||||
extensions: {
|
||||
http: {
|
||||
status: 403,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(shouldCaptureException(graphQLError)).toBe(false);
|
||||
});
|
||||
|
||||
it('should not capture throttler limit reached exceptions', () => {
|
||||
const throttlerException = new ThrottlerException(
|
||||
'Limit reached',
|
||||
ThrottlerExceptionCode.LIMIT_REACHED,
|
||||
);
|
||||
|
||||
expect(shouldCaptureException(throttlerException)).toBe(false);
|
||||
});
|
||||
|
||||
it('should capture unexpected errors', () => {
|
||||
expect(shouldCaptureException(new Error('Unexpected'))).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -18,6 +18,10 @@ import {
|
||||
TimeoutError,
|
||||
ValidationError,
|
||||
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
import {
|
||||
ThrottlerException,
|
||||
ThrottlerExceptionCode,
|
||||
} from 'src/engine/core-modules/throttler/throttler.exception';
|
||||
import { type CustomException } from 'src/utils/custom-exception';
|
||||
|
||||
const graphQLPredefinedExceptions = {
|
||||
@@ -80,6 +84,13 @@ export const shouldCaptureException = (
|
||||
return false;
|
||||
}
|
||||
|
||||
if (
|
||||
exception instanceof ThrottlerException &&
|
||||
exception.code === ThrottlerExceptionCode.LIMIT_REACHED
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (statusCode && statusCode < 500) {
|
||||
return false;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user