Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(shorebird_cli): don't fail if a release artifact has already been uploaded #554

Merged
merged 3 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ ${summary.join('\n')}
}
}

// TODO(bryanoltman): Consolidate aab and other artifact creation.
// TODO(bryanoltman): Parallelize artifact creation.
final createArtifactProgress = logger.progress('Creating artifacts');
for (final archMetadata in architectures.values) {
final artifactPath = p.join(
Expand All @@ -236,6 +238,13 @@ ${summary.join('\n')}
platform: platform,
hash: hash,
);
} on CodePushConflictException catch (_) {
// Newlines are due to how logger.info interacts with logger.progress.
logger.info(
'''

${archMetadata.arch} artifact already exists, continuing...''',
);
} catch (error) {
createArtifactProgress.fail('Error uploading ${artifact.path}: $error');
return ExitCode.software.code;
Expand All @@ -250,6 +259,13 @@ ${summary.join('\n')}
platform: platform,
hash: _hashFn(await File(bundlePath).readAsBytes()),
);
} on CodePushConflictException catch (_) {
// Newlines are due to how logger.info interacts with logger.progress.
logger.info(
'''

aab artifact already exists, continuing...''',
);
} catch (error) {
createArtifactProgress.fail('Error uploading $bundlePath: $error');
return ExitCode.software.code;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,66 @@ Did you forget to run "shorebird init"?''',
expect(exitCode, ExitCode.software.code);
});

test('logs message when uploading release artifact that already exists.',
() async {
const error = 'something went wrong';
when(
() => codePushClient.getReleases(appId: any(named: 'appId')),
).thenAnswer((_) async => []);
when(
() => codePushClient.createReleaseArtifact(
artifactPath: any(named: 'artifactPath'),
releaseId: any(named: 'releaseId'),
arch: any(named: 'arch'),
platform: any(named: 'platform'),
hash: any(named: 'hash'),
),
).thenThrow(const CodePushConflictException(message: error));
final tempDir = setUpTempDir();
setUpTempArtifacts(tempDir);
final exitCode = await IOOverrides.runZoned(
command.run,
getCurrentDirectory: () => tempDir,
);

// 1 for each arch, 1 for the aab
final numArtifactsUploaded = Arch.values.length + 1;
verify(
() => logger.info(any(that: contains('already exists'))),
).called(numArtifactsUploaded);
verifyNever(() => progress.fail(error));
expect(exitCode, ExitCode.success.code);
});

test('logs message when uploading aab that already exists.', () async {
const error = 'something went wrong';
when(
() => codePushClient.getReleases(appId: any(named: 'appId')),
).thenAnswer((_) async => []);
when(
() => codePushClient.createReleaseArtifact(
artifactPath: any(named: 'artifactPath', that: endsWith('.aab')),
releaseId: any(named: 'releaseId'),
arch: any(named: 'arch'),
platform: any(named: 'platform'),
hash: any(named: 'hash'),
),
).thenThrow(const CodePushConflictException(message: error));
final tempDir = setUpTempDir();
setUpTempArtifacts(tempDir);
final exitCode = await IOOverrides.runZoned(
command.run,
getCurrentDirectory: () => tempDir,
);
verify(
() => logger.info(
any(that: contains('aab artifact already exists, continuing...')),
),
).called(1);
verifyNever(() => progress.fail(error));
expect(exitCode, ExitCode.success.code);
});

test('throws error when uploading release artifact fails.', () async {
const error = 'something went wrong';
when(
Expand Down
62 changes: 39 additions & 23 deletions packages/shorebird_code_push_client/lib/src/code_push_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ class CodePushException implements Exception {
String toString() => '$message${details != null ? '\n$details' : ''}';
}

/// {@template code_push_conflict_exception}
/// Exception thrown when a 409 response is received.
/// {@endtemplate}
class CodePushConflictException extends CodePushException {
/// {@macro code_push_conflict_exception}
const CodePushConflictException({required super.message, super.details});
}

/// {@template code_push_client}
/// Dart client for the Shorebird CodePush API.
/// {@endtemplate}
Expand Down Expand Up @@ -54,7 +62,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.created) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}
}

Expand All @@ -66,7 +74,7 @@ class CodePushClient {
if (response.statusCode == HttpStatus.notFound) {
return null;
} else if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}

final json = jsonDecode(response.body) as Map<String, dynamic>;
Expand Down Expand Up @@ -96,7 +104,9 @@ class CodePushClient {
final response = await _httpClient.send(request);
final body = await response.stream.bytesToString();

if (response.statusCode != HttpStatus.ok) throw _parseErrorResponse(body);
if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.statusCode, body);
}

return PatchArtifact.fromJson(json.decode(body) as Map<String, dynamic>);
}
Expand All @@ -108,7 +118,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}

return CreatePaymentLinkResponse.fromJson(
Expand Down Expand Up @@ -139,7 +149,9 @@ class CodePushClient {
final response = await _httpClient.send(request);
final body = await response.stream.bytesToString();

if (response.statusCode != HttpStatus.ok) throw _parseErrorResponse(body);
if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.statusCode, body);
}

return ReleaseArtifact.fromJson(json.decode(body) as Map<String, dynamic>);
}
Expand All @@ -153,7 +165,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}
final body = json.decode(response.body) as Map<String, dynamic>;
return App.fromJson(body);
Expand All @@ -170,7 +182,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}
final body = json.decode(response.body) as Map<String, dynamic>;
return Channel.fromJson(body);
Expand All @@ -184,7 +196,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}

final body = json.decode(response.body) as Map<String, dynamic>;
Expand All @@ -209,7 +221,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}
final body = json.decode(response.body) as Map<String, dynamic>;
return Release.fromJson(body);
Expand All @@ -225,7 +237,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.noContent) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}
}

Expand All @@ -236,7 +248,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.noContent) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}
}

Expand All @@ -252,7 +264,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.created) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}

final body = json.decode(response.body) as Json;
Expand All @@ -266,7 +278,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.noContent) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}
}

Expand All @@ -275,7 +287,7 @@ class CodePushClient {
final response = await _httpClient.get(Uri.parse('$_v1/apps'));

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}

final apps = json.decode(response.body) as List;
Expand All @@ -293,7 +305,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}

final channels = json.decode(response.body) as List;
Expand All @@ -309,7 +321,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}

final releases = json.decode(response.body) as List;
Expand All @@ -329,7 +341,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}

final releases = json.decode(response.body) as List;
Expand All @@ -354,7 +366,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}

final body = json.decode(response.body) as Map<String, dynamic>;
Expand All @@ -372,7 +384,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.created) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}
}

Expand All @@ -383,7 +395,7 @@ class CodePushClient {
);

if (response.statusCode != HttpStatus.ok) {
throw _parseErrorResponse(response.body);
throw _parseErrorResponse(response.statusCode, response.body);
}

final json = jsonDecode(response.body) as Map<String, dynamic>;
Expand All @@ -394,14 +406,18 @@ class CodePushClient {
/// Closes the client.
void close() => _httpClient.close();

CodePushException _parseErrorResponse(String response) {
CodePushException _parseErrorResponse(int statusCode, String response) {
final exceptionBuilder = statusCode == HttpStatus.conflict
? CodePushConflictException.new
: CodePushException.new;

final ErrorResponse error;
try {
final body = json.decode(response) as Map<String, dynamic>;
error = ErrorResponse.fromJson(body);
} catch (_) {
throw const CodePushException(message: unknownErrorMessage);
throw exceptionBuilder(message: unknownErrorMessage);
}
return CodePushException(message: error.message, details: error.details);
return exceptionBuilder(message: error.message, details: error.details);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,32 @@ void main() {
);
});

test(
'throws a CodePushConflictException if the http response code is 409',
() {
when(() => httpClient.send(any())).thenAnswer((_) async {
return http.StreamedResponse(
Stream.value(utf8.encode(json.encode(errorResponse.toJson()))),
HttpStatus.conflict,
);
});

final tempDir = Directory.systemTemp.createTempSync();
final fixture = File(path.join(tempDir.path, 'release.txt'))
..createSync();

expect(
codePushClient.createReleaseArtifact(
artifactPath: fixture.path,
releaseId: releaseId,
arch: arch,
platform: platform,
hash: hash,
),
throwsA(isA<CodePushConflictException>()),
);
});

test('throws an exception if the http request fails', () async {
when(() => httpClient.send(any())).thenAnswer((_) async {
return http.StreamedResponse(
Expand Down