Skip to content

Commit a49b326

Browse files
authored
Merge pull request #238 from open-runtimes/fix-silent-build-command-failure
Report silent build command failures
2 parents fee06ee + bdc5ff0 commit a49b326

5 files changed

Lines changed: 64 additions & 4 deletions

File tree

app/config/errors.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@
2424
'message' => 'You are not authorized to access this resource.',
2525
'code' => 401,
2626
],
27+
/* Build */
28+
Exception::BUILD_FAILED => [
29+
'name' => Exception::BUILD_FAILED,
30+
'short' => 'Build failed',
31+
'message' => 'Build failed.',
32+
'code' => 400,
33+
'publish' => false,
34+
],
2735
/* Runtime */
2836
Exception::RUNTIME_FAILED => [
2937
'name' => Exception::RUNTIME_FAILED,

src/Executor/Exception.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ class Exception extends \RuntimeException
2424

2525
public const string EXECUTION_BAD_JSON = 'execution_bad_json';
2626

27+
public const string BUILD_FAILED = 'build_failed';
28+
2729
public const string RUNTIME_NOT_FOUND = 'runtime_not_found';
2830

2931
public const string RUNTIME_CONFLICT = 'runtime_conflict';

src/Executor/Runner/Docker.php

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,9 @@ public function createRuntime(
376376
* Execute any commands if they were provided
377377
*/
378378
if ($command !== '' && $command !== '0') {
379+
$stdout = '';
380+
379381
try {
380-
$stdout = '';
381382
$status = $this->orchestration->execute(
382383
name: $runtimeName,
383384
command: $this->getBuildCommands($command, $version),
@@ -386,7 +387,12 @@ public function createRuntime(
386387
);
387388

388389
if (!$status) {
389-
throw new ExecutorException(ExecutorException::RUNTIME_FAILED, 'Failed to create runtime: ' . $stdout);
390+
$message = \trim(\mb_strcut($stdout, 0, MAX_BUILD_LOG_SIZE));
391+
392+
throw new ExecutorException(
393+
ExecutorException::BUILD_FAILED,
394+
$message === '' ? 'Build command failed.' : $message
395+
);
390396
}
391397

392398
if ($version === 'v2') {
@@ -398,6 +404,16 @@ public function createRuntime(
398404
} elseif (!$cacheEnabled) {
399405
$output = Logs::get($runtimeName);
400406
}
407+
} catch (ExecutorException $err) {
408+
throw $err;
409+
} catch (OrchestrationException $err) {
410+
$message = \trim(\mb_strcut($stdout, 0, MAX_BUILD_LOG_SIZE));
411+
412+
throw new ExecutorException(
413+
ExecutorException::BUILD_FAILED,
414+
$message === '' ? 'Build command failed.' : $message,
415+
previous: $err
416+
);
401417
} catch (Throwable $err) {
402418
throw new ExecutorException(ExecutorException::RUNTIME_FAILED, $err->getMessage(), null, $err);
403419
}
@@ -506,6 +522,10 @@ public function createRuntime(
506522
$localDevice->deletePath($tmpFolder);
507523
$this->runtimes->remove($runtimeName);
508524

525+
if ($throwable instanceof ExecutorException) {
526+
throw $throwable;
527+
}
528+
509529
$message = '';
510530
foreach ($output as $chunk) {
511531
$message .= $chunk['content'];
@@ -545,14 +565,14 @@ private function getBuildCommands(string $command, string $version): array
545565
return [
546566
'sh',
547567
'-c',
548-
'touch /var/tmp/logs.txt && (' . $command . ') >> /var/tmp/logs.txt 2>&1 && cat /var/tmp/logs.txt'
568+
'touch /var/tmp/logs.txt && (' . $command . ') >> /var/tmp/logs.txt 2>&1; status=$?; cat /var/tmp/logs.txt; if [ $status -ne 0 ]; then echo "Build command exited with code $status."; fi; exit $status'
549569
];
550570
}
551571

552572
return [
553573
'bash',
554574
'-c',
555-
'mkdir -p /tmp/logging && touch /tmp/logging/timings.txt && touch /tmp/logging/logs.txt && script --log-out /tmp/logging/logs.txt --flush --log-timing /tmp/logging/timings.txt --return --quiet --command "' . \str_replace('"', '\"', $command) . '"'
575+
'mkdir -p /tmp/logging && touch /tmp/logging/timings.txt && touch /tmp/logging/logs.txt && script --log-out /tmp/logging/logs.txt --flush --log-timing /tmp/logging/timings.txt --return --quiet --command "' . \str_replace('"', '\"', $command) . '"; status=$?; if [ $status -ne 0 ]; then echo "Build command exited with code $status."; fi; exit $status'
556576
];
557577
}
558578

tests/e2e/ExecutorTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,23 @@ public function testBuild(): void
299299

300300
$response = $this->client->call(Client::METHOD_POST, '/runtimes', [], $params);
301301
$this->assertEquals(400, $response['headers']['status-code']);
302+
$this->assertEquals('build_failed', $response['body']['type']);
303+
304+
/** Silent user error in build command */
305+
$params = [
306+
'runtimeId' => 'test-build-fail-silent-' . $runtimeId,
307+
'source' => '/storage/functions/php/code.tar.gz',
308+
'destination' => '/storage/builds/test',
309+
'entrypoint' => 'index.php',
310+
'image' => 'openruntimes/php:v5-8.1',
311+
'command' => 'exit 1',
312+
'remove' => true
313+
];
314+
315+
$response = $this->client->call(Client::METHOD_POST, '/runtimes', [], $params);
316+
$this->assertEquals(400, $response['headers']['status-code']);
317+
$this->assertEquals('build_failed', $response['body']['type']);
318+
$this->assertStringContainsString('Build command exited with code 1.', (string) $response['body']['message']);
302319

303320
/** Invalid cache key */
304321
$params = [

tests/unit/Executor/Runner/DockerTest.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,19 @@ public function testNoCacheLifecycleShellCommandsAreGenerated(): void
4444
$this->assertStringNotContainsString('build-cache-save.sh', $command);
4545
}
4646

47+
public function testBuildCommandsReportSilentNonZeroExit(): void
48+
{
49+
$docker = $this->createDocker();
50+
51+
foreach (['v2', 'v5'] as $version) {
52+
$commands = $this->invokeGetBuildCommands($docker, 'exit 1', $version);
53+
$command = \implode(' ', $commands);
54+
55+
$this->assertStringContainsString('Build command exited with code $status.', $command);
56+
$this->assertStringContainsString('exit $status', $command);
57+
}
58+
}
59+
4760
public function testInvalidBuildCacheKeyIsRejected(): void
4861
{
4962
$this->expectException(\Exception::class);

0 commit comments

Comments
 (0)