Don't impose a strict size limit to uploads (#4014)
diff --git a/lib/src/authentication/client.dart b/lib/src/authentication/client.dart
index c36e529..ffa7889 100644
--- a/lib/src/authentication/client.dart
+++ b/lib/src/authentication/client.dart
@@ -11,6 +11,7 @@
import '../http.dart';
import '../log.dart' as log;
import '../system_cache.dart';
+import '../utils.dart';
import 'credential.dart';
/// This client authenticates requests by injecting `Authentication` header to
@@ -83,11 +84,7 @@
}
}
if (serverMessage != null) {
- // Only allow printable ASCII, map anything else to whitespace, take
- // at-most 1024 characters.
- serverMessage = String.fromCharCodes(
- serverMessage.runes.map((r) => 32 <= r && r <= 127 ? r : 32).take(1024),
- );
+ serverMessage = sanitizeForTerminal(serverMessage);
}
throw AuthenticationException(response.statusCode, serverMessage);
}
diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart
index 9c4372c..8b63718 100644
--- a/lib/src/command/lish.dart
+++ b/lib/src/command/lish.dart
@@ -179,9 +179,7 @@
} on PubHttpResponseException catch (error) {
var url = error.response.request!.url;
if (url == cloudStorageUrl) {
- // TODO(nweiz): the response may have XML-formatted information about
- // the error. Try to parse that out once we have an easily-accessible
- // XML parser.
+ handleGCSError(error.response);
fail(log.red('Failed to upload the package.'));
} else if (Uri.parse(url.origin) == Uri.parse(host.origin)) {
handleJsonError(error.response);
@@ -277,14 +275,17 @@
'${tree.fromFiles(files, baseDir: entrypoint.rootDir, showFileSizes: true)}',
);
- var packageBytesFuture =
- createTarGz(files, baseDir: entrypoint.rootDir).toBytes();
+ var packageBytes =
+ await createTarGz(files, baseDir: entrypoint.rootDir).toBytes();
+ log.message(
+ '\nTotal compressed archive size: ${_readableFileSize(packageBytes.length)}.\n',
+ );
// Validate the package.
var isValid = skipValidation
? true
: await _validate(
- packageBytesFuture.then((bytes) => bytes.length),
+ packageBytes.length,
files,
);
if (!isValid) {
@@ -294,7 +295,7 @@
log.message('The server may enforce additional checks.');
return;
} else {
- await _publish(await packageBytesFuture);
+ await _publish(packageBytes);
}
}
@@ -307,7 +308,7 @@
/// Validates the package. Completes to false if the upload should not
/// proceed.
- Future<bool> _validate(Future<int> packageSize, List<String> files) async {
+ Future<bool> _validate(int packageSize, List<String> files) async {
final hints = <String>[];
final warnings = <String>[];
final errors = <String>[];
@@ -368,3 +369,15 @@
return true;
}
}
+
+String _readableFileSize(int size) {
+ if (size >= 1 << 30) {
+ return '${size ~/ (1 << 30)} GB';
+ } else if (size >= 1 << 20) {
+ return '${size ~/ (1 << 20)} MB';
+ } else if (size >= 1 << 10) {
+ return '${size ~/ (1 << 10)} KB';
+ } else {
+ return '<1 KB';
+ }
+}
diff --git a/lib/src/http.dart b/lib/src/http.dart
index 294d567..d05c512 100644
--- a/lib/src/http.dart
+++ b/lib/src/http.dart
@@ -211,6 +211,41 @@
fail(log.red(error['message'] as String));
}
+/// Handles an unsuccessful XML-formatted response from google cloud storage.
+///
+/// Assumes messages are of the form in
+/// https://cloud.google.com/storage/docs/xml-api/reference-status
+///
+/// This is a poor person's XML parsing with regexps, but this should be
+/// sufficient for the specified messages.
+void handleGCSError(http.BaseResponse response) {
+ if (response is http.Response) {
+ final responseBody = response.body;
+ if (responseBody.contains('<?xml')) {
+ String? getTagText(String tag) {
+ final result = RegExp('<$tag>(.*)</$tag>').firstMatch(responseBody)?[1];
+ if (result == null) return null;
+ return sanitizeForTerminal(result);
+ }
+
+ final code = getTagText('Code');
+ // TODO(sigurdm): we could hard-code nice error messages for known codes.
+ final message = getTagText('Message');
+ // `Details` are not specified in the doc above, but have been observed in actual responses.
+ final details = getTagText('Details');
+ if (code != null) {
+ log.error('Server error code: $code');
+ }
+ if (message != null) {
+ log.error('Server message: $message');
+ }
+ if (details != null) {
+ log.error('Server details: $details');
+ }
+ }
+ }
+}
+
/// Parses a response body, assuming it's JSON-formatted.
///
/// Throws a user-friendly error if the response body is invalid JSON, or if
diff --git a/lib/src/utils.dart b/lib/src/utils.dart
index 31ca1d1..d43780a 100644
--- a/lib/src/utils.dart
+++ b/lib/src/utils.dart
@@ -767,3 +767,13 @@
String option(String name) => this[name] as String;
String? optionWithoutDefault(String name) => this[name] as String?;
}
+
+/// Limits the range of characters and length.
+///
+/// Useful for displaying externally provided strings.
+///
+/// Only allowd printable ASCII, map anything else to whitespace, take at-most
+/// 1024 characters.
+String sanitizeForTerminal(String input) => String.fromCharCodes(
+ input.runes.map((r) => 32 <= r && r <= 127 ? r : 32).take(1024),
+ );
diff --git a/lib/src/validator.dart b/lib/src/validator.dart
index 07ef9b3..ba923df 100644
--- a/lib/src/validator.dart
+++ b/lib/src/validator.dart
@@ -128,7 +128,7 @@
/// upload to the server.
static Future<void> runAll(
Entrypoint entrypoint,
- Future<int> packageSize,
+ int packageSize,
Uri serverUrl,
List<String> files, {
required List<String> hints,
@@ -163,7 +163,7 @@
final context = ValidationContext(
entrypoint,
- await packageSize,
+ packageSize,
serverUrl,
files,
);
diff --git a/lib/src/validator/size.dart b/lib/src/validator/size.dart
index ae32496..a8a7eae 100644
--- a/lib/src/validator/size.dart
+++ b/lib/src/validator/size.dart
@@ -7,7 +7,7 @@
import '../io.dart';
import '../validator.dart';
-/// The maximum size of the package to upload (100 MB).
+/// The preferred maximum size of the package to upload (100 MB).
const _maxSize = 100 * 1024 * 1024;
/// A validator that validates that a package isn't too big.
@@ -19,17 +19,19 @@
// Current implementation of Package.listFiles skips hidden files
var ignoreExists = fileExists(entrypoint.root.path('.gitignore'));
- var error = StringBuffer('Your package is $sizeInMb MB. Hosted '
- 'packages must be smaller than 100 MB.');
+ var hint = StringBuffer('''
+Your package is $sizeInMb MB.
+
+Consider the impact large downloads can have on the package consumer.''');
if (ignoreExists && !entrypoint.root.inGitRepo) {
- error.write(' Your .gitignore has no effect since your project '
+ hint.write('\nYour .gitignore has no effect since your project '
'does not appear to be in version control.');
} else if (!ignoreExists && entrypoint.root.inGitRepo) {
- error.write(' Consider adding a .gitignore to avoid including '
+ hint.write('\nConsider adding a .gitignore to avoid including '
'temporary files.');
}
- errors.add(error.toString());
+ hints.add(hint.toString());
}
}
diff --git a/test/lish/cloud_storage_upload_provides_an_error_test.dart b/test/lish/cloud_storage_upload_provides_an_error_test.dart
index bd14d84..a11ba95 100644
--- a/test/lish/cloud_storage_upload_provides_an_error_test.dart
+++ b/test/lish/cloud_storage_upload_provides_an_error_test.dart
@@ -22,14 +22,30 @@
globalServer.expect('POST', '/upload', (request) {
return request.read().drain().then((_) {
return shelf.Response.notFound(
- '<Error><Message>Your request sucked.</Message></Error>',
+ // Actual example of an error code we get from GCS
+ "<?xml version='1.0' encoding='UTF-8'?><Error><Code>EntityTooLarge</Code><Message>Your proposed upload is larger than the maximum object size specified in your Policy Document.</Message><Details>Content-length exceeds upper bound on range</Details></Error>",
headers: {'content-type': 'application/xml'},
);
});
});
- // TODO(nweiz): This should use the server's error message once the client
- // can parse the XML.
+ expect(
+ pub.stderr,
+ emits(
+ 'Server error code: EntityTooLarge',
+ ),
+ );
+ expect(
+ pub.stderr,
+ emits(
+ 'Server message: Your proposed upload is larger than the maximum object size specified in your Policy Document.',
+ ),
+ );
+ expect(
+ pub.stderr,
+ emits('Server details: Content-length exceeds upper bound on range'),
+ );
+
expect(pub.stderr, emits('Failed to upload the package.'));
await pub.shouldExit(1);
});
diff --git a/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt b/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt
index e4da8be..f463833 100644
--- a/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt
+++ b/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt
@@ -118,6 +118,8 @@
├── lib
│ └── test_pkg.dart (<1 KB)
└── pubspec.yaml (<1 KB)
+
+Total compressed archive size: <1 KB.
Validating package...
The server may enforce additional checks.
[STDERR]
diff --git a/test/testdata/goldens/lish/many_files_test/displays all files.txt b/test/testdata/goldens/lish/many_files_test/displays all files.txt
index 82a9431..2d37456 100644
--- a/test/testdata/goldens/lish/many_files_test/displays all files.txt
+++ b/test/testdata/goldens/lish/many_files_test/displays all files.txt
@@ -29,6 +29,8 @@
│ ├── file_9.dart (<1 KB)
│ └── test_pkg.dart (<1 KB)
└── pubspec.yaml (<1 KB)
+
+Total compressed archive size: <1 KB.
Validating package...
Publishing is forever; packages cannot be unpublished.
diff --git a/test/validator/size_test.dart b/test/validator/size_test.dart
index 02a093c..f04245a 100644
--- a/test/validator/size_test.dart
+++ b/test/validator/size_test.dart
@@ -11,30 +11,29 @@
import '../test_pub.dart';
import 'utils.dart';
-Future<void> expectSizeValidationError(Matcher matcher) async {
+Future<void> expectSizeValidationHint(Matcher matcher) async {
await expectValidationDeprecated(
SizeValidator.new,
size: 100 * (1 << 20) + 1,
- errors: contains(matcher),
+ hints: contains(matcher),
);
}
void main() {
- test('considers a package valid if it is <= 100 MB', () async {
+ test('ho hint if package is <= 100 MB', () async {
await d.validPackage().create();
await expectValidationDeprecated(SizeValidator.new, size: 100);
await expectValidationDeprecated(SizeValidator.new, size: 100 * (1 << 20));
});
- group('considers a package invalid if it is more than 100 MB', () {
+ group('hints if package is more than 100 MB', () {
test('package is not under source control and no .gitignore exists',
() async {
await d.validPackage().create();
- await expectSizeValidationError(
- equals('Your package is 100.0 MB. Hosted packages must '
- 'be smaller than 100 MB.'),
+ await expectSizeValidationHint(
+ contains('Your package is 100.0 MB.'),
);
});
@@ -42,9 +41,9 @@
await d.validPackage().create();
await d.dir(appPath, [d.file('.gitignore', 'ignored')]).create();
- await expectSizeValidationError(
+ await expectSizeValidationHint(
allOf(
- contains('Hosted packages must be smaller than 100 MB.'),
+ contains('Your package is 100.0 MB.'),
contains('Your .gitignore has no effect since your project '
'does not appear to be in version control.'),
),
@@ -55,9 +54,9 @@
await d.validPackage().create();
await d.git(appPath).create();
- await expectSizeValidationError(
+ await expectSizeValidationHint(
allOf(
- contains('Hosted packages must be smaller than 100 MB.'),
+ contains('Your package is 100.0 MB.'),
contains('Consider adding a .gitignore to avoid including '
'temporary files.'),
),
@@ -68,9 +67,8 @@
await d.validPackage().create();
await d.git(appPath, [d.file('.gitignore', 'ignored')]).create();
- await expectSizeValidationError(
- equals('Your package is 100.0 MB. Hosted packages must '
- 'be smaller than 100 MB.'),
+ await expectSizeValidationHint(
+ contains('Your package is 100.0 MB.'),
);
});
});
