1
0
mirror of https://github.com/laurent22/joplin.git synced 2025-11-26 22:41:17 +02:00

All: Improved S3 sync error handling and reliability, and upgraded S3 SDK (#5312)

This commit is contained in:
Lee Matos
2021-11-24 18:03:03 -05:00
committed by GitHub
parent 8e54a65ca5
commit 5981227c06
16 changed files with 35302 additions and 23496 deletions

View File

@@ -4,7 +4,7 @@ const Setting = require('./models/Setting').default;
const { FileApi } = require('./file-api.js');
const Synchronizer = require('./Synchronizer').default;
const { FileApiDriverAmazonS3 } = require('./file-api-driver-amazon-s3.js');
const S3 = require('aws-sdk/clients/s3');
const { S3Client, HeadBucketCommand } = require('@aws-sdk/client-s3');
class SyncTargetAmazonS3 extends BaseSyncTarget {
static id() {
@@ -25,7 +25,7 @@ class SyncTargetAmazonS3 extends BaseSyncTarget {
}
static label() {
return `${_('AWS S3')} (Beta)`;
return `${_('S3')} (Beta)`;
}
static description() {
@@ -40,12 +40,17 @@ class SyncTargetAmazonS3 extends BaseSyncTarget {
return Setting.value('sync.8.path');
}
// These are the settings that get read from disk to instantiate the API.
s3AuthParameters() {
return {
accessKeyId: Setting.value('sync.8.username'),
secretAccessKey: Setting.value('sync.8.password'),
s3UseArnRegion: true, // override the request region with the region inferred from requested resource's ARN
s3ForcePathStyle: true,
// We need to set a region. See https://github.com/aws/aws-sdk-js-v3/issues/1845#issuecomment-754832210
region: Setting.value('sync.8.region'),
credentials: {
accessKeyId: Setting.value('sync.8.username'),
secretAccessKey: Setting.value('sync.8.password'),
},
UseArnRegion: true, // override the request region with the region inferred from requested resource's ARN.
forcePathStyle: Setting.value('sync.8.forcePathStyle'), // Older implementations may not support more modern access, so we expose this to allow people the option to toggle.
endpoint: Setting.value('sync.8.url'),
};
}
@@ -53,26 +58,45 @@ class SyncTargetAmazonS3 extends BaseSyncTarget {
api() {
if (this.api_) return this.api_;
this.api_ = new S3(this.s3AuthParameters());
this.api_ = new S3Client(this.s3AuthParameters());
// There is a bug with auto skew correction in aws-sdk-js-v3
// and this attempts to remove the skew correction for all calls.
// There are some additional spots in the app where we reset this
// to zero as well as it appears the skew logic gets triggered
// which makes "RequestTimeTooSkewed" errors...
// See https://github.com/aws/aws-sdk-js-v3/issues/2208
this.api_.config.systemClockOffset = 0;
return this.api_;
}
static async newFileApi_(syncTargetId, options) {
// These options are read from the form on the page
// so we can test new config choices without overriding the current settings.
const apiOptions = {
accessKeyId: options.username(),
secretAccessKey: options.password(),
s3UseArnRegion: true,
s3ForcePathStyle: true,
region: options.region(),
credentials: {
accessKeyId: options.username(),
secretAccessKey: options.password(),
},
UseArnRegion: true, // override the request region with the region inferred from requested resource's ARN.
forcePathStyle: options.forcePathStyle(),
endpoint: options.url(),
};
const api = new S3(apiOptions);
const api = new S3Client(apiOptions);
const driver = new FileApiDriverAmazonS3(api, SyncTargetAmazonS3.s3BucketName());
const fileApi = new FileApi('', driver);
fileApi.setSyncTargetId(syncTargetId);
return fileApi;
}
// With the aws-sdk-v3-js some errors (301/403) won't get their XML parsed properly.
// I think it's this issue: https://github.com/aws/aws-sdk-js-v3/issues/1596
// If you save the config on desktop, restart the app and attempt a sync, we should get a clearer error message because the sync logic has more robust XML error parsing.
// We could implement that here, but the above workaround saves some code.
static async checkConfig(options) {
const fileApi = await SyncTargetAmazonS3.newFileApi_(SyncTargetAmazonS3.id(), options);
fileApi.requestRepeatCount_ = 0;
@@ -81,22 +105,28 @@ class SyncTargetAmazonS3 extends BaseSyncTarget {
ok: false,
errorMessage: '',
};
try {
const headBucketReq = new Promise((resolve, reject) => {
fileApi.driver().api().headBucket({
Bucket: options.path(),
},(err, response) => {
if (err) reject(err);
else resolve(response);
});
fileApi.driver().api().send(
new HeadBucketCommand({
Bucket: options.path(),
}),(err, response) => {
if (err) reject(err);
else resolve(response);
});
});
const result = await headBucketReq;
if (!result) throw new Error(`AWS S3 bucket not found: ${SyncTargetAmazonS3.s3BucketName()}`);
output.ok = true;
} catch (error) {
output.errorMessage = error.message;
if (error.code) output.errorMessage += ` (Code ${error.code})`;
if (error.message) {
output.errorMessage = error.message;
}
if (error.code) {
output.errorMessage += ` (Code ${error.code})`;
}
}
return output;