Skip to content

Commit 1c510f4

Browse files
BluePsyduckBluePsyduck
committed
Improved error message in case Factorio crashes.
1 parent cb31f14 commit 1c510f4

File tree

5 files changed

+273
-9
lines changed

5 files changed

+273
-9
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
### Changed
66

77
- Extracted icon renderer to Go-based binary for major performance improvement.
8+
- Improved error message in the case that Factorio crashed.
89

910
### Fixed
1011

‎src/Exception/FactorioExecutionException.php

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace FactorioItemBrowser\Export\Exception;
6+
7+
use Throwable;
8+
9+
/**
10+
* The exception thrown when the execution of Factorio failed.
11+
*
12+
* @author BluePsyduck <[email protected]>
13+
* @license http://opensource.org/licenses/GPL-3.0 GPL v3
14+
*/
15+
class FactorioExecutionException extends ExportException
16+
{
17+
/**
18+
* The message template of the exception.
19+
*/
20+
protected const MESSAGE = 'Factorio exited with code %d: %s';
21+
22+
/**
23+
* The regular expression to detect the line starting the error.
24+
*/
25+
protected const REGEXP_ERROR_LINE = '(^[ ]*\d+\.\d{3} Error |^Error: )';
26+
27+
/**
28+
* Initializes the exception.
29+
* @param int $exitCode
30+
* @param string $output
31+
* @param Throwable|null $previous
32+
*/
33+
public function __construct(int $exitCode, string $output, ?Throwable $previous = null)
34+
{
35+
$message = $this->extractErrorMessageFromOutput($output);
36+
parent::__construct(sprintf(self::MESSAGE, $exitCode, $message), $exitCode, $previous);
37+
}
38+
39+
/**
40+
* Extracts the actual error message from the output.
41+
* @param string $output
42+
* @return string
43+
*/
44+
protected function extractErrorMessageFromOutput(string $output): string
45+
{
46+
$errorLines = [];
47+
$errorFound = false;
48+
49+
$lines = array_reverse(explode(PHP_EOL, $output));
50+
foreach ($lines as $line) {
51+
$errorLines[] = $line;
52+
if (preg_match(self::REGEXP_ERROR_LINE, $line) > 0) {
53+
$errorFound = true;
54+
break;
55+
}
56+
}
57+
58+
if (!$errorFound) {
59+
// We were unable to detect the start of the error. So take the last 10 lines instead.
60+
$errorLines = [];
61+
foreach (array_slice($lines, 0, 10) as $line) {
62+
if (strpos($line, '>>>') !== false) {
63+
// We have our dump placeholder, so break now before we add the dump to the message.
64+
break;
65+
}
66+
$errorLines[] = $line;
67+
}
68+
}
69+
70+
return implode(PHP_EOL, array_reverse($errorLines));
71+
}
72+
}

‎src/Factorio/Instance.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use FactorioItemBrowser\Export\Entity\Dump\Dump;
1010
use FactorioItemBrowser\Export\Entity\InfoJson;
1111
use FactorioItemBrowser\Export\Exception\ExportException;
12+
use FactorioItemBrowser\Export\Exception\FactorioExecutionException;
1213
use FactorioItemBrowser\Export\Mod\ModFileManager;
1314
use JMS\Serializer\SerializerInterface;
1415
use Symfony\Component\Process\Process;
@@ -177,6 +178,7 @@ protected function createDumpInfoJson(array $modNames): InfoJson
177178

178179
$info = new InfoJson();
179180
$info->setName('Dump')
181+
->setTitle('Factorio Item Browser - Dump')
180182
->setAuthor('factorio-item-browser')
181183
->setVersion('1.0.0')
182184
->setFactorioVersion($baseInfo->getVersion())
@@ -188,11 +190,16 @@ protected function createDumpInfoJson(array $modNames): InfoJson
188190
/**
189191
* Executes the Factorio instance.
190192
* @return string
193+
* @throws ExportException
191194
*/
192195
protected function execute(): string
193196
{
194197
$process = $this->createProcess();
195198
$process->run();
199+
if (!$process->isSuccessful()) {
200+
throw new FactorioExecutionException((int) $process->getExitCode(), $process->getOutput());
201+
}
202+
196203
return $process->getOutput();
197204
}
198205

‎test/src/Exception/FactorioExecutionExceptionTest.php

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace FactorioItemBrowserTest\Export\Exception;
6+
7+
use BluePsyduck\TestHelper\ReflectionTrait;
8+
use Exception;
9+
use FactorioItemBrowser\Export\Exception\FactorioExecutionException;
10+
use PHPUnit\Framework\MockObject\MockObject;
11+
use PHPUnit\Framework\TestCase;
12+
use ReflectionException;
13+
14+
/**
15+
* The PHPUnit test of the FactorioExecutionException class.
16+
*
17+
* @author BluePsyduck <[email protected]>
18+
* @license http://opensource.org/licenses/GPL-3.0 GPL v3
19+
* @coversDefaultClass \FactorioItemBrowser\Export\Exception\FactorioExecutionException
20+
*/
21+
class FactorioExecutionExceptionTest extends TestCase
22+
{
23+
use ReflectionTrait;
24+
25+
/**
26+
* Tests the constructing.
27+
* @covers ::__construct
28+
*/
29+
public function testConstruct(): void
30+
{
31+
$output = 'abc';
32+
$errorMessage = 'def';
33+
$exitCode = 42;
34+
$expectedMessage = 'Factorio exited with code 42: def';
35+
36+
/* @var Exception&MockObject $previous */
37+
$previous = $this->createMock(Exception::class);
38+
39+
/* @var FactorioExecutionException&MockObject $exception */
40+
$exception = $this->getMockBuilder(FactorioExecutionException::class)
41+
->onlyMethods(['extractErrorMessageFromOutput'])
42+
->disableOriginalConstructor()
43+
->getMock();
44+
$exception->expects($this->once())
45+
->method('extractErrorMessageFromOutput')
46+
->with($this->identicalTo($output))
47+
->willReturn($errorMessage);
48+
49+
$exception->__construct($exitCode, $output, $previous);
50+
51+
$this->assertSame($expectedMessage, $exception->getMessage());
52+
$this->assertSame($exitCode, $exception->getCode());
53+
$this->assertSame($previous, $exception->getPrevious());
54+
}
55+
56+
/**
57+
* Provides the data for the extractErrorMessageFromOutput test.
58+
* @return array<mixed>
59+
*/
60+
public function provideExtractErrorMessageFromOutput(): array
61+
{
62+
$output1 = <<<EOT
63+
0.007 Running in headless mode
64+
0.009 Loading mod core 0.0.0 (data.lua)
65+
0.061 Loading mod base 0.18.21 (data.lua)
66+
0.235 Error Util.cpp:83: Failed to load mod "base": __base__/data.lua:89: __base__/prototypes/...
67+
stack traceback:
68+
[C]: in function 'require'
69+
__base__/data.lua:89: in main chunk
70+
EOT;
71+
$message1 = <<<EOT
72+
0.235 Error Util.cpp:83: Failed to load mod "base": __base__/data.lua:89: __base__/prototypes/...
73+
stack traceback:
74+
[C]: in function 'require'
75+
__base__/data.lua:89: in main chunk
76+
EOT;
77+
78+
$output2 = <<<EOT
79+
2.399 Checksum for script /project/data/instances/2f4a45fa-a509-a9d1-aae6-ffcf984a7a76/...
80+
2.401 Checksum for script __Dump__/control.lua: 3285258963
81+
Error: The mod Factorio Item Browser - Dump (1.0.0) caused a non-recoverable error.
82+
Please report this error to the mod author.
83+
84+
Error while running event Dump::on_init()
85+
__Dump__/map.lua:82: attempt to index global 'prototype2' (a nil value)
86+
stack traceback:
87+
__Dump__/map.lua:82: in function 'recipe'
88+
__Dump__/control.lua:37: in function <__Dump__/control.lua:4>
89+
2.448 Goodbye
90+
EOT;
91+
$message2 = <<<EOT
92+
Error: The mod Factorio Item Browser - Dump (1.0.0) caused a non-recoverable error.
93+
Please report this error to the mod author.
94+
95+
Error while running event Dump::on_init()
96+
__Dump__/map.lua:82: attempt to index global 'prototype2' (a nil value)
97+
stack traceback:
98+
__Dump__/map.lua:82: in function 'recipe'
99+
__Dump__/control.lua:37: in function <__Dump__/control.lua:4>
100+
2.448 Goodbye
101+
EOT;
102+
103+
$output3 = <<<EOT
104+
Lorem ipsum dolor sit amet.
105+
>>>TEST>>>Actual dumped data we do not want to see in the error<<<TEST<<<
106+
foo
107+
bar
108+
2.448 Goodbye
109+
EOT;
110+
$message3 = <<<EOT
111+
foo
112+
bar
113+
2.448 Goodbye
114+
EOT;
115+
116+
return [
117+
[$output1, $message1],
118+
[$output2, $message2],
119+
[$output3, $message3],
120+
];
121+
}
122+
123+
/**
124+
* Tests the extractErrorMessageFromOutput method.
125+
* @param string $output
126+
* @param string $expectedResult
127+
* @throws ReflectionException
128+
* @covers ::extractErrorMessageFromOutput
129+
* @dataProvider provideExtractErrorMessageFromOutput
130+
*/
131+
public function testExtractErrorMessageFromOutput(string $output, string $expectedResult): void
132+
{
133+
/* @var FactorioExecutionException&MockObject $exception */
134+
$exception = $this->getMockBuilder(FactorioExecutionException::class)
135+
->onlyMethods(['__construct'])
136+
->disableOriginalConstructor()
137+
->getMock();
138+
139+
$result = $this->invokeMethod($exception, 'extractErrorMessageFromOutput', $output);
140+
141+
$this->assertSame($expectedResult, $result);
142+
}
143+
}

‎test/src/Factorio/InstanceTest.php

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use FactorioItemBrowser\Export\Entity\Dump\Dump;
1111
use FactorioItemBrowser\Export\Entity\InfoJson;
1212
use FactorioItemBrowser\Export\Exception\ExportException;
13+
use FactorioItemBrowser\Export\Exception\FactorioExecutionException;
1314
use FactorioItemBrowser\Export\Factorio\DumpExtractor;
1415
use FactorioItemBrowser\Export\Factorio\Instance;
1516
use FactorioItemBrowser\Export\Mod\ModFileManager;
@@ -247,6 +248,7 @@ public function testCreateDumpInfoJson(): void
247248

248249
$expectedResult = new InfoJson();
249250
$expectedResult->setName('Dump')
251+
->setTitle('Factorio Item Browser - Dump')
250252
->setAuthor('factorio-item-browser')
251253
->setVersion('1.0.0')
252254
->setFactorioVersion($baseVersion)
@@ -274,18 +276,18 @@ public function testExecute(): void
274276
{
275277
$output = 'abc';
276278

277-
/* @var Process|MockObject $process */
278-
$process = $this->getMockBuilder(Process::class)
279-
->onlyMethods(['run', 'getOutput'])
280-
->disableOriginalConstructor()
281-
->getMock();
279+
/* @var Process&MockObject $process */
280+
$process = $this->createMock(Process::class);
282281
$process->expects($this->once())
283282
->method('run');
283+
$process->expects($this->once())
284+
->method('isSuccessful')
285+
->willReturn(true);
284286
$process->expects($this->once())
285287
->method('getOutput')
286288
->willReturn($output);
287289

288-
/* @var Instance|MockObject $instance */
290+
/* @var Instance&MockObject $instance */
289291
$instance = $this->getMockBuilder(Instance::class)
290292
->onlyMethods(['createProcess'])
291293
->disableOriginalConstructor()
@@ -299,6 +301,45 @@ public function testExecute(): void
299301
$this->assertSame($output, $result);
300302
}
301303

304+
/**
305+
* Tests the execute method.
306+
* @throws ReflectionException
307+
* @covers ::execute
308+
*/
309+
public function testExecuteWithException(): void
310+
{
311+
$output = 'abc';
312+
$exitCode = 42;
313+
314+
/* @var Process&MockObject $process */
315+
$process = $this->createMock(Process::class);
316+
$process->expects($this->once())
317+
->method('run');
318+
$process->expects($this->once())
319+
->method('isSuccessful')
320+
->willReturn(false);
321+
$process->expects($this->once())
322+
->method('getExitCode')
323+
->willReturn($exitCode);
324+
$process->expects($this->once())
325+
->method('getOutput')
326+
->willReturn($output);
327+
328+
$this->expectException(FactorioExecutionException::class);
329+
$this->expectExceptionCode($exitCode);
330+
331+
/* @var Instance&MockObject $instance */
332+
$instance = $this->getMockBuilder(Instance::class)
333+
->onlyMethods(['createProcess'])
334+
->disableOriginalConstructor()
335+
->getMock();
336+
$instance->expects($this->once())
337+
->method('createProcess')
338+
->willReturn($process);
339+
340+
$this->invokeMethod($instance, 'execute');
341+
}
342+
302343
/**
303344
* Tests the createProcess method.
304345
* @throws ReflectionException
@@ -308,7 +349,7 @@ public function testCreateProcess(): void
308349
{
309350
$expectedCommandLine = "'abc' '--no-log-rotation' '--create=def' '--mod-directory=ghi'";
310351

311-
/* @var Instance|MockObject $instance */
352+
/* @var Instance&MockObject $instance */
312353
$instance = $this->getMockBuilder(Instance::class)
313354
->onlyMethods(['getInstancePath'])
314355
->disableOriginalConstructor()
@@ -341,7 +382,7 @@ public function testCreateDirectory(): void
341382
{
342383
$directory = vfsStream::setup('root');
343384

344-
/* @var Instance|MockObject $instance */
385+
/* @var Instance&MockObject $instance */
345386
$instance = $this->getMockBuilder(Instance::class)
346387
->onlyMethods(['getInstancePath'])
347388
->disableOriginalConstructor()
@@ -377,7 +418,7 @@ public function testCopy(): void
377418
$factorioPath = vfsStream::url('root/factorio/abc');
378419
$instancePath = vfsStream::url('root/instance/abc');
379420

380-
/* @var Instance|MockObject $instance */
421+
/* @var Instance&MockObject $instance */
381422
$instance = $this->getMockBuilder(Instance::class)
382423
->onlyMethods(['getFactorioPath', 'getInstancePath'])
383424
->disableOriginalConstructor()

0 commit comments

Comments
 (0)