merging backend logic #252
Conversation
BREAKING CHANGE: Replaced direct cloud storage configuration with pre-signed URLs - Added pre-signed URL endpoint configuration - Removed cloud storage credentials from frontend - Deleted cloud storage config files - Updated upload logic to use pre-signed URLs - Simplified configuration interface
- Added CORS mode to uploadObject fetch request - Prettier fixes BREAKING CHANGE: Requires S3 bucket CORS configuration
- Added client-side SDK approach for cloud storage providers - Implemented AWS integration with presigned URLs - Added credential refresh and SAS token management - Implemented progress tracking and retry mechanisms - Fixed type errors and improve error handling - Removed unused code and align with latest SDK versions BREAKING CHANGE: Storage configuration requires provider-specific SDK setup
- Added client-side SDK approach for Azure provider - Implemented Azure integration with presigned URLs - Implemented progress tracking - Removed unused code and align with latest SDK versions
- Storage config only requires `provider` and `tokenEndpoint` values
- Added client-side SDK approach for DO provider - Implemented DO integration with presigned URLs
- Unified s3 client-side upload approach for S3 complaint storage providers
- preview url for uploaded files
- made ETag header conditional - removed `previewUrl` from object returned to FE
- changed folder structure to accommodate BE logic
src/frontend/lib/uploadObject.ts
Outdated
| method: 'PUT', | ||
| body: fields ? formData : file, | ||
| headers: { | ||
| 'Content-Type': file.type, |
There was a problem hiding this comment.
When sending FormData, manually setting the 'Content-Type' header interferes with the automatic setting of the boundary parameter, causing the request to fail.
Recommendation: Remove the 'Content-Type' header when fields are present and FormData is used. Let the browser set it automatically. Modify the code as follows:
const response = await fetch(presignedUrl, {
method: 'PUT',
body: fields ? formData : file,
headers: fields
? undefined // Do not set 'Content-Type' header when sending FormData
: {
'Content-Type': file.type,
},
mode: 'cors',
credentials: 'include',
})
| return true | ||
| } | ||
|
|
||
| private handleError(error: unknown): never { |
There was a problem hiding this comment.
Issue: The method relies on checking substrings in err.message to determine the error type. This approach is fragile and can break if error messages change or are localized.
Recommendation: Use more structured error handling by checking specific error properties like name, code, or custom properties returned from the backend. For example, if the backend returns an error object with a code property, you can use that to determine the error type:
private handleError(error: unknown): never {
if (error instanceof UploadError) throw error;
const err = error as any;
if (err.code === 'UNAUTHORIZED') {
throw new UploadError(
'Unauthorized access to Provider',
UploadErrorType.PERMISSION_ERROR
);
}
if (err.code === 'EXPIRED_URL') {
throw new UploadError(
'Presigned URL has expired',
UploadErrorType.EXPIRED_URL,
true
);
}
throw new UploadError(
`Upload failed: ${err.message}`,
UploadErrorType.UNKNOWN_UPLOAD_ERROR,
true
);
}
| }) | ||
| } | ||
|
|
||
| validateConfig(): boolean { |
There was a problem hiding this comment.
Validation of Required Configurations:
Issue: The validateConfig method only checks for tokenEndpoint, but provider is also critical for the functionality.
Recommendation: Update the validateConfig method to include provider:
validateConfig(): boolean {
const required = ['tokenEndpoint', 'provider'] as const;
const missing = required.filter(key => !this.config[key]);
if (missing.length > 0) {
throw new Error(`Missing required configuration: ${missing.join(', ')}`);
}
// Optionally, validate that provider is a valid enum value
if (!Object.values(Provider).includes(this.config.provider)) {
throw new Error(`Invalid provider: ${this.config.provider}`);
}
return true;
}
src/frontend/lib/storage/provider.ts
Outdated
| }) | ||
|
|
||
| xhr.addEventListener('error', () => | ||
| reject(new Error('Network error during upload')), |
There was a problem hiding this comment.
Recommendation: Enhance error handling by including xhr.status and xhr.statusText:
xhr.addEventListener('error', () =>
reject(
new Error(
`Network error during upload: ${xhr.status} ${xhr.statusText}`,
),
),
);
- undid setting `Content-Type ` header when sending fetch request from `uploadObject` - improved logic for `handleError` - improved validation of required configuration - Enhanced error handling by including `xhr.status` and `xhr.statusText` when reporting error from `uploadWithProgress` - fixed issue with build failing when temp dist directory exists
3 participants
No description provided.