Skip to content

Commit 29a742f

Browse files
adamzieladamziel
committed
Throw an exception on error, introduce error domains to handle them safely
1 parent b35b3e4 commit 29a742f

13 files changed

+220
-180
lines changed

‎src/WordPress/ByteReader/WP_Byte_Reader.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,12 @@ abstract public function seek( int $offset ): bool;
1212
abstract public function is_finished(): bool;
1313
abstract public function next_bytes(): bool;
1414
abstract public function get_bytes(): ?string;
15-
abstract public function get_last_error(): ?string;
1615
abstract public function close(): bool;
1716
public function read_all(): string {
1817
$buffer = '';
1918
while( $this->next_bytes() ) {
2019
$buffer .= $this->get_bytes();
2120
}
22-
if( $this->get_last_error() ) {
23-
return false;
24-
}
2521
return $buffer;
2622
}
2723
}

‎src/WordPress/ByteReader/WP_File_Reader.php‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace WordPress\ByteReader;
44

5+
use WordPress\Error\WordPressException;
6+
57
class WP_File_Reader extends WP_Byte_Reader {
68

79
const STATE_STREAMING = '#streaming';
@@ -13,19 +15,14 @@ class WP_File_Reader extends WP_Byte_Reader {
1315
protected $offset_in_file;
1416
protected $output_bytes = '';
1517
protected $last_chunk_size = 0;
16-
protected $last_error;
1718
protected $state = self::STATE_STREAMING;
1819

1920
static public function create( $file_path, $chunk_size = 8096 ) {
2021
if(!file_exists($file_path)) {
21-
throw new \Exception(sprintf( 'File %s does not exist', $file_path ));
22-
_doing_it_wrong( __METHOD__, sprintf( 'File %s does not exist', $file_path ), '1.0.0' );
23-
return false;
22+
throw new WordPressException(sprintf( 'File %s does not exist', $file_path ));
2423
}
2524
if(!is_file($file_path)) {
26-
throw new \Exception(sprintf( '%s is not a file', $file_path ));
27-
_doing_it_wrong( __METHOD__, sprintf( '%s is not a file', $file_path ), '1.0.0' );
28-
return false;
25+
throw new WordPressException(sprintf( '%s is not a file', $file_path ));
2926
}
3027
return new self( $file_path, $chunk_size );
3128
}
@@ -48,8 +45,7 @@ public function tell(): int {
4845

4946
public function seek( $offset_in_file ): bool {
5047
if ( ! is_int( $offset_in_file ) ) {
51-
_doing_it_wrong( __METHOD__, 'Cannot set a file reader cursor to a non-integer offset.', '1.0.0' );
52-
return false;
48+
throw new WordPressException('Cannot set a file reader cursor to a non-integer offset.');
5349
}
5450
$this->offset_in_file = $offset_in_file;
5551
$this->last_chunk_size = 0;
@@ -64,11 +60,10 @@ public function seek( $offset_in_file ): bool {
6460

6561
public function close(): bool {
6662
if(!$this->file_pointer) {
67-
return false;
63+
throw new WordPressException('File pointer is not open');
6864
}
6965
if(!fclose($this->file_pointer)) {
70-
$this->last_error = 'Failed to close file pointer';
71-
return false;
66+
throw new WordPressException('Failed to close file pointer');
7267
}
7368
$this->file_pointer = null;
7469
$this->state = static::STATE_FINISHED;
@@ -83,18 +78,17 @@ public function get_bytes(): string {
8378
return $this->output_bytes;
8479
}
8580

86-
public function get_last_error(): ?string {
87-
return $this->last_error;
88-
}
89-
9081
public function next_bytes(): bool {
9182
$this->output_bytes = '';
9283
$this->last_chunk_size = 0;
93-
if ( $this->last_error || $this->is_finished() ) {
84+
if ( $this->is_finished() ) {
9485
return false;
9586
}
9687
if ( ! $this->file_pointer ) {
9788
$this->file_pointer = fopen( $this->file_path, 'r' );
89+
if(false === $this->file_pointer) {
90+
throw new WordPressException(sprintf('Failed to open the file: %s', $this->file_path));
91+
}
9892
if ( $this->offset_in_file ) {
9993
fseek( $this->file_pointer, $this->offset_in_file );
10094
}

‎src/WordPress/ByteReader/WP_GZ_File_Reader.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ class WP_GZ_File_Reader extends WP_File_Reader {
66

77
public function next_bytes(): bool {
88
$this->output_bytes = '';
9-
if ( $this->last_error || $this->is_finished() ) {
9+
if ( $this->is_finished() ) {
1010
return false;
1111
}
1212
if ( ! $this->file_pointer ) {

‎src/WordPress/ByteReader/WP_Remote_File_Ranged_Reader.php‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,6 @@ static private function redirect_output_to_disk( WP_Byte_Reader $reader ) {
9494
fwrite($file, $reader->get_bytes());
9595
}
9696
fclose($file);
97-
if($reader->get_last_error()) {
98-
// How should we log this error?
99-
return false;
100-
}
10197
return WP_File_Reader::create( $file_path );
10298
}
10399

@@ -185,12 +181,6 @@ public function get_bytes(): ?string {
185181
return $this->current_reader->get_bytes();
186182
}
187183

188-
public function get_last_error(): ?string {
189-
// @TODO: Preserve the error information when the current reader
190-
// is reset.
191-
return $this->current_reader->get_last_error();
192-
}
193-
194184
private function ensure_content_length() {
195185
if ( null !== $this->remote_file_length ) {
196186
return $this->remote_file_length;

‎src/WordPress/ByteReader/WP_Remote_File_Reader.php‎

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace WordPress\ByteReader;
44

5+
use WordPress\Error\WordPressException;
6+
57
/**
68
* Streams bytes from a remote file.
79
*/
@@ -33,13 +35,10 @@ public function tell(): int {
3335

3436
public function seek( $offset_in_file ): bool {
3537
if ( $this->request ) {
36-
_doing_it_wrong(
37-
__METHOD__,
38+
throw new WordPressException(
3839
'Cannot seek() a WP_Remote_File_Reader instance once the request was initialized. ' .
39-
'Use WP_Remote_File_Ranged_Reader to seek() using range requests instead.',
40-
'1.0.0'
40+
'Use WP_Remote_File_Ranged_Reader to seek() using range requests instead.'
4141
);
42-
return false;
4342
}
4443
$this->skip_bytes = $offset_in_file;
4544
return true;
@@ -52,8 +51,7 @@ public function next_bytes(): bool {
5251
array( 'headers' => $this->headers )
5352
);
5453
if ( false === $this->client->enqueue( $this->request ) ) {
55-
// TODO: Think through error handling
56-
return false;
54+
throw new WordPressException(sprintf('Failed to enqueue the request to %s', $this->url));
5755
}
5856
}
5957

@@ -85,8 +83,7 @@ public function next_bytes(): bool {
8583
case \WordPress\AsyncHttp\Client::EVENT_BODY_CHUNK_AVAILABLE:
8684
$chunk = $this->client->get_response_body_chunk();
8785
if ( ! is_string( $chunk ) ) {
88-
// TODO: Think through error handling
89-
return false;
86+
throw new WordPressException(sprintf('Failed to get the response body chunk from %s', $this->url));
9087
}
9188
$this->current_chunk = $chunk;
9289

@@ -108,11 +105,7 @@ public function next_bytes(): bool {
108105
}
109106
return true;
110107
case \WordPress\AsyncHttp\Client::EVENT_FAILED:
111-
// TODO: Think through error handling. Errors are expected when working with
112-
// the network. Should we auto retry? Make it easy for the caller to retry?
113-
// Something else?
114-
$this->last_error = $this->client->get_request()->error;
115-
return false;
108+
throw new WordPressException(sprintf('Failed to fetch data from %s', $this->url));
116109
case \WordPress\AsyncHttp\Client::EVENT_FINISHED:
117110
$this->is_finished = true;
118111
return false;
@@ -130,23 +123,16 @@ public function length(): ?int {
130123
array( 'method' => 'HEAD' )
131124
);
132125
if ( false === $this->client->enqueue( $request ) ) {
133-
// TODO: Think through error handling
134-
return false;
126+
throw new WordPressException(sprintf('Failed to enqueue the request to %s', $this->url));
135127
}
136128
while ( $this->client->await_next_event() ) {
137129
switch ( $this->client->get_event() ) {
138130
case \WordPress\AsyncHttp\Client::EVENT_GOT_HEADERS:
139131
$request = $this->client->get_request();
140-
if ( ! $request ) {
141-
return false;
142-
}
143132
if($request->redirected_to) {
144133
continue 2;
145134
}
146135
$response = $request->response;
147-
if ( false === $response ) {
148-
return false;
149-
}
150136
$content_length = $response->get_header( 'Content-Length' );
151137
if ( false === $content_length ) {
152138
return false;
@@ -181,11 +167,6 @@ public function is_finished(): bool {
181167
}
182168

183169
public function close(): bool {
184-
_doing_it_wrong(
185-
__METHOD__,
186-
'Not implemented yet',
187-
'1.0.0'
188-
);
189-
return false;
170+
throw new WordPressException('Not implemented yet');
190171
}
191172
}

‎src/WordPress/ByteReader/WP_String_Reader.php‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace WordPress\ByteReader;
44

5+
use WordPress\Error\WordPressException;
6+
57
class WP_String_Reader extends WP_Byte_Reader {
68

79
const STATE_STREAMING = '#streaming';
@@ -12,13 +14,11 @@ class WP_String_Reader extends WP_Byte_Reader {
1214
protected $offset = 0;
1315
protected $output_bytes = '';
1416
protected $last_chunk_size = 0;
15-
protected $last_error;
1617
protected $state = self::STATE_STREAMING;
1718

1819
static public function create($string, $chunk_size = 8096) {
1920
if (!is_string($string)) {
20-
_doing_it_wrong(__METHOD__, 'Input must be a string', '1.0.0');
21-
return false;
21+
throw new WordPressException('Input must be a string');
2222
}
2323
return new self($string, $chunk_size);
2424
}
@@ -38,11 +38,10 @@ public function tell(): int {
3838

3939
public function seek($offset): bool {
4040
if (!is_int($offset)) {
41-
_doing_it_wrong(__METHOD__, 'Cannot set cursor to a non-integer offset.', '1.0.0');
42-
return false;
41+
throw new WordPressException('Cannot set cursor to a non-integer offset.');
4342
}
4443
if ($offset < 0 || $offset > strlen($this->string)) {
45-
return false;
44+
throw new WordPressException('Cannot set cursor to an offset outside the string bounds.');
4645
}
4746
$this->offset = $offset;
4847
$this->last_chunk_size = 0;
@@ -63,15 +62,11 @@ public function get_bytes(): string {
6362
return $this->output_bytes;
6463
}
6564

66-
public function get_last_error(): ?string {
67-
return $this->last_error;
68-
}
69-
7065
public function next_bytes(): bool {
7166
$this->output_bytes = '';
7267
$this->last_chunk_size = 0;
7368

74-
if ($this->last_error || $this->is_finished()) {
69+
if ($this->is_finished()) {
7570
return false;
7671
}
7772

‎src/WordPress/Error/ErrorDomain.php‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
<?php
2+
3+
namespace WordPress\Error;
4+
5+
use \WP_Error;
6+
7+
/**
8+
* A Domain groups error handling into a container, similar to Node.js domains.
9+
* It allows grouping multiple different operations into one error handling context.
10+
*/
11+
class ErrorDomain {
12+
13+
/**
14+
* @var callable|null The error handler registered for this domain
15+
*/
16+
private $error_handler = null;
17+
18+
/**
19+
* @var string|null A label for the domain, useful for debugging
20+
*/
21+
private $label = null;
22+
23+
/**
24+
* @var callable|null A fallback fallback error handler
25+
*/
26+
private static $fallback_error_handler = null;
27+
28+
private $last_return_value = null;
29+
30+
/**
31+
* Creates a new domain instance with an optional label.
32+
*
33+
* @param string|null $label A label for the domain
34+
* @return self
35+
*/
36+
public static function create($label = null) {
37+
$instance = new self();
38+
$instance->label = $label;
39+
return $instance;
40+
}
41+
42+
/**
43+
* Sets a fallback fallback error handler.
44+
*
45+
* @param callable $handler The fallback error handler function
46+
* @return void
47+
*/
48+
public static function set_fallback_error_handler($handler) {
49+
self::$fallback_error_handler = $handler;
50+
}
51+
52+
public static function bail($exception) {
53+
throw $exception;
54+
}
55+
56+
/**
57+
* Adds an error handler to this domain.
58+
*
59+
* @param callable $handler The error handler function
60+
* @return void
61+
*/
62+
public function set_error_handler($handler) {
63+
$this->error_handler = $handler;
64+
}
65+
66+
/**
67+
* Executes a callback within this domain's error handling context.
68+
*
69+
* @param callable $callback The code to execute
70+
* @return bool True if the callback executed successfully, false otherwise
71+
*/
72+
public function safe_run(callable $callback) {
73+
try {
74+
$this->last_return_value = $callback($this);
75+
return true;
76+
} catch (\Throwable $exception) {
77+
if ($this->error_handler) {
78+
call_user_func($this->error_handler, $exception, $this);
79+
} else {
80+
call_user_func(self::$fallback_error_handler, $exception);
81+
}
82+
return false;
83+
}
84+
}
85+
86+
public function get_last_return_value() {
87+
return $this->last_return_value;
88+
}
89+
90+
/**
91+
* Gets the label of the domain.
92+
*
93+
* @return string|null The label of the domain
94+
*/
95+
public function get_label() {
96+
return $this->label;
97+
}
98+
99+
}
100+
101+
// Set the default fallback error handler
102+
ErrorDomain::set_fallback_error_handler(function($exception) {
103+
error_log('fallback handler: ' . $exception->getMessage() . "\n" . $exception->getTraceAsString());
104+
});

‎src/WordPress/Error/WordPressException.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace WordPress\Error;
4+
5+
class WordPressException extends \Exception {
6+
7+
}

0 commit comments

Comments
 (0)