Skip to content

Commit 86cdead

Browse files
bshafferbshaffer
authored
feat: add better handling for exceptions (GoogleCloudPlatform#62)
1 parent 40c3f5f commit 86cdead

8 files changed

+193
-94
lines changed

‎src/CloudEventFunctionWrapper.php

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
use GuzzleHttp\Psr7\Response;
2121
use Psr\Http\Message\ResponseInterface;
2222
use Psr\Http\Message\ServerRequestInterface;
23-
use LogicException;
24-
use RuntimeException;
2523

2624
class CloudEventFunctionWrapper extends FunctionWrapper
2725
{
@@ -42,23 +40,49 @@ class CloudEventFunctionWrapper extends FunctionWrapper
4240

4341
public function execute(ServerRequestInterface $request): ResponseInterface
4442
{
43+
$body = (string) $request->getBody();
44+
$eventType = $this->getEventType($request);
45+
// We expect JSON if the content-type ends in "json" or if the event
46+
// type is legacy or structured Cloud Event.
47+
$shouldValidateJson = in_array($eventType, [
48+
self::TYPE_LEGACY,
49+
self::TYPE_STRUCTURED
50+
]) || 'json' === substr($request->getHeaderLine('content-type'), -4);
51+
52+
if ($shouldValidateJson) {
53+
$data = json_decode($body, true);
54+
55+
// Validate JSON, return 400 Bad Request on error
56+
if (json_last_error() != JSON_ERROR_NONE) {
57+
return new Response(400, [
58+
self::FUNCTION_STATUS_HEADER => 'crash'
59+
], sprintf(
60+
'Could not parse CloudEvent: %s',
61+
'' !== $body ? json_last_error_msg() : 'Missing cloudevent payload'
62+
));
63+
}
64+
} else {
65+
$data = $body;
66+
}
67+
4568
switch ($this->getEventType($request)) {
4669
case self::TYPE_LEGACY:
4770
$mapper = new LegacyEventMapper();
48-
$cloudevent = $mapper->fromRequest($request);
71+
$cloudevent = $mapper->fromJsonData($data);
4972
break;
5073

5174
case self::TYPE_STRUCTURED:
52-
$cloudevent = $this->fromStructuredRequest($request);
75+
$cloudevent = CloudEvent::fromArray($data);
5376
break;
5477

5578
case self::TYPE_BINARY:
56-
$cloudevent = $this->fromBinaryRequest($request);
79+
$cloudevent = $this->fromBinaryRequest($request, $data);
5780
break;
5881

5982
default:
60-
throw new LogicException('Invalid event type');
61-
break;
83+
return new Response(400, [
84+
self::FUNCTION_STATUS_HEADER => 'crash'
85+
], 'invalid event type');
6286
}
6387

6488
call_user_func($this->function, $cloudevent);
@@ -81,34 +105,10 @@ private function getEventType(ServerRequestInterface $request)
81105
}
82106
}
83107

84-
private function parseJsonData(ServerRequestInterface $request)
85-
{
86-
// Get Body
87-
$body = (string) $request->getBody();
88-
89-
$jsonData = json_decode($body, true);
90-
if (json_last_error() != JSON_ERROR_NONE) {
91-
throw new RuntimeException(sprintf(
92-
'Could not parse CloudEvent: %s',
93-
'' !== $body ? json_last_error_msg() : 'Missing cloudevent payload'
94-
));
95-
}
96-
97-
return $jsonData;
98-
}
99-
100-
private function fromStructuredRequest(
101-
ServerRequestInterface $request
102-
): CloudEvent {
103-
$jsonData = $this->parseJsonData($request);
104-
return CloudEvent::fromArray($jsonData);
105-
}
106-
107108
private function fromBinaryRequest(
108-
ServerRequestInterface $request
109+
ServerRequestInterface $request,
110+
string $data
109111
): CloudEvent {
110-
$jsonData = $this->parseJsonData($request);
111-
112112
$content = [];
113113

114114
foreach (self::$validKeys as $key) {
@@ -117,7 +117,7 @@ private function fromBinaryRequest(
117117
$content[$key] = $request->getHeaderLine($ceKey);
118118
}
119119
}
120-
$content['data'] = $jsonData;
120+
$content['data'] = $data;
121121
return CloudEvent::fromArray($content);
122122
}
123123

‎src/FunctionWrapper.php

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

2828
abstract class FunctionWrapper
2929
{
30+
const FUNCTION_STATUS_HEADER = 'X-Google-Status';
31+
3032
protected $function;
3133

3234
public function __construct(callable $function, array $signature = null)
@@ -36,8 +38,6 @@ public function __construct(callable $function, array $signature = null)
3638
);
3739

3840
$this->function = $function;
39-
40-
// TODO: validate function signature, if present.
4141
}
4242

4343
abstract public function execute(

‎src/HttpFunctionWrapper.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,14 @@ public function execute(ServerRequestInterface $request): ResponseInterface
3838
$response = call_user_func($this->function, $request);
3939

4040
if (is_string($response)) {
41-
$response = new Response(200, [], $response);
41+
return new Response(200, [], $response);
42+
} elseif ($response instanceof ResponseInterface) {
43+
return $response;
4244
}
4345

44-
return $response;
46+
throw new \LogicException(
47+
'Function response must be string or ' . ResponseInterface::class
48+
);
4549
}
4650

4751
protected function getFunctionParameterClassName(): string

‎src/Invoker.php

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,15 @@
1818
namespace Google\CloudFunctions;
1919

2020
use InvalidArgumentException;
21+
use GuzzleHttp\Psr7\Response;
2122
use GuzzleHttp\Psr7\ServerRequest;
2223
use Psr\Http\Message\ResponseInterface;
2324
use Psr\Http\Message\ServerRequestInterface;
2425

2526
class Invoker
2627
{
2728
private $function;
29+
private $errorLogFunc;
2830

2931
/**
3032
* @param $target callable The callable to be invoked
@@ -44,6 +46,12 @@ public function __construct(callable $target, string $signatureType)
4446
throw new InvalidArgumentException(sprintf(
4547
'Invalid signature type: "%s"', $signatureType));
4648
}
49+
$this->errorLogFunc = function (string $error) {
50+
fwrite(fopen('php://stderr', 'wb'), json_encode([
51+
'message' => $error,
52+
'severity' => 'error'
53+
], JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES));
54+
};
4755
}
4856

4957
public function handle(
@@ -53,6 +61,19 @@ public function handle(
5361
$request = ServerRequest::fromGlobals();
5462
}
5563

56-
return $this->function->execute($request);
64+
try {
65+
return $this->function->execute($request);
66+
} catch (\Exception $e) {
67+
// Log the full error and stack trace
68+
($this->errorLogFunc)((string) $e);
69+
// Set "X-Google-Status" to "crash" for Http functions and "error"
70+
// for Cloud Events
71+
$statusHeader = $this->function instanceof HttpFunctionWrapper
72+
? 'crash'
73+
: 'error';
74+
return new Response(500, [
75+
FunctionWrapper::FUNCTION_STATUS_HEADER => $statusHeader,
76+
]);
77+
}
5778
}
5879
}

‎src/LegacyEventMapper.php

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,11 @@
1717

1818
namespace Google\CloudFunctions;
1919

20-
use Psr\Http\Message\ServerRequestInterface;
21-
use RuntimeException;
22-
2320
class LegacyEventMapper
2421
{
25-
public function fromRequest(
26-
ServerRequestInterface $request
27-
): CloudEvent {
28-
list($context, $data) = $this->getLegacyEventContextAndData($request);
22+
public function fromJsonData(array $jsonData): CloudEvent
23+
{
24+
list($context, $data) = $this->getLegacyEventContextAndData($jsonData);
2925

3026
$eventType = $context->getEventType();
3127
$resourceName = $context->getResourceName();
@@ -58,27 +54,8 @@ public function fromRequest(
5854
]);
5955
}
6056

61-
private function parseJsonData(ServerRequestInterface $request)
57+
private function getLegacyEventContextAndData(array $jsonData): array
6258
{
63-
// Get Body
64-
$body = (string) $request->getBody();
65-
66-
$jsonData = json_decode($body, true);
67-
if (json_last_error() != JSON_ERROR_NONE) {
68-
throw new RuntimeException(sprintf(
69-
'Could not parse request body: %s',
70-
'' !== $body ? json_last_error_msg() : 'Missing event payload'
71-
));
72-
}
73-
74-
return $jsonData;
75-
}
76-
77-
private function getLegacyEventContextAndData(
78-
ServerRequestInterface $request
79-
): array {
80-
$jsonData = $this->parseJsonData($request);
81-
8259
$data = $jsonData['data'] ?? null;
8360

8461
if (array_key_exists('context', $jsonData)) {

‎tests/CloudEventFunctionsWrapperTest.php

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,65 @@ public function setUp(): void
3737

3838
public function testInvalidCloudEventRequestBody()
3939
{
40-
$this->expectException('RuntimeException');
41-
$this->expectExceptionMessage('Could not parse CloudEvent: Syntax error');
4240
$headers = ['content-type' => 'application/cloudevents+json'];
4341
$request = new ServerRequest('POST', '/', $headers, 'notjson');
4442
$cloudEventFunctionWrapper = new CloudEventFunctionWrapper([$this, 'invokeThis']);
45-
$cloudEventFunctionWrapper->execute($request);
43+
$response = $cloudEventFunctionWrapper->execute($request);
44+
$this->assertEquals(400, $response->getStatusCode());
45+
$this->assertEquals(
46+
'Could not parse CloudEvent: Syntax error',
47+
(string) $response->getBody()
48+
);
49+
$this->assertEquals('crash', $response->getHeaderLine('X-Google-Status'));
4650
}
4751

4852
public function testInvalidLegacyEventRequestBody()
4953
{
50-
$this->expectException('RuntimeException');
51-
$this->expectExceptionMessage('Could not parse request body: Syntax error');
5254
$request = new ServerRequest('POST', '/', [], 'notjson');
5355
$cloudEventFunctionWrapper = new CloudEventFunctionWrapper([$this, 'invokeThis']);
54-
$cloudEventFunctionWrapper->execute($request);
56+
$response = $cloudEventFunctionWrapper->execute($request);
57+
$this->assertEquals(400, $response->getStatusCode());
58+
$this->assertEquals(
59+
'Could not parse CloudEvent: Syntax error',
60+
(string) $response->getBody()
61+
);
62+
$this->assertEquals('crash', $response->getHeaderLine('X-Google-Status'));
63+
}
64+
65+
public function testNonJsonIsValidInBinaryCloudEventRequestBody()
66+
{
67+
$request = new ServerRequest('POST', '/', [
68+
'ce-id' => 'fooBar',
69+
'ce-source' => 'my-source',
70+
'ce-specversion' => '1.0',
71+
'ce-type' => 'my.type',
72+
], 'notjson');
73+
$cloudEventFunctionWrapper = new CloudEventFunctionWrapper(
74+
[$this, 'invokeThisPartial']
75+
);
76+
$response = $cloudEventFunctionWrapper->execute($request);
77+
$this->assertEquals(200, $response->getStatusCode());
78+
}
79+
80+
public function testInvalidJsonBinaryCloudEventRequestBody()
81+
{
82+
$request = new ServerRequest('POST', '/', [
83+
'ce-id' => 'fooBar',
84+
'ce-source' => 'my-source',
85+
'ce-specversion' => '1.0',
86+
'ce-type' => 'my.type',
87+
'Content-Type' => 'application/json',
88+
], 'notjson');
89+
$cloudEventFunctionWrapper = new CloudEventFunctionWrapper(
90+
[$this, 'invokeThisPartial']
91+
);
92+
$response = $cloudEventFunctionWrapper->execute($request);
93+
$this->assertEquals(400, $response->getStatusCode());
94+
$this->assertEquals(
95+
'Could not parse CloudEvent: Syntax error',
96+
(string) $response->getBody()
97+
);
98+
$this->assertEquals('crash', $response->getHeaderLine('X-Google-Status'));
5599
}
56100

57101
public function testNoFunctionParameters()

‎tests/InvokerTest.php

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,13 @@
1717

1818
namespace Google\CloudFunctions\Tests;
1919

20+
use Google\CloudFunctions\CloudEvent;
2021
use Google\CloudFunctions\Invoker;
22+
use Google\CloudFunctions\FunctionWrapper;
23+
use GuzzleHttp\Psr7\ServerRequest;
2124
use PHPUnit\Framework\TestCase;
2225
use Psr\Http\Message\ServerRequestInterface;
26+
use ReflectionClass;
2327

2428
/**
2529
* @group gcf-framework
@@ -40,8 +44,67 @@ public function testHttpInvoker()
4044
$this->assertEquals((string) $response->getBody(), 'Invoked!');
4145
}
4246

47+
/**
48+
* @dataProvider provideErrorHandling
49+
*/
50+
public function testErrorHandling($signatureType, $errorStatus, $request = null)
51+
{
52+
$functionName = sprintf('invoke%sError', ucwords($signatureType));
53+
$invoker = new Invoker([$this, $functionName], $signatureType);
54+
// use a custom error log func
55+
$message = null;
56+
$newErrorLogFunc = function (string $error) use (&$message) {
57+
$message = $error;
58+
};
59+
$errorLogFuncProp = (new ReflectionClass($invoker))
60+
->getProperty('errorLogFunc');
61+
$errorLogFuncProp->setAccessible(true);
62+
$errorLogFuncProp->setValue($invoker, $newErrorLogFunc);
63+
64+
// Invoke the handler
65+
$response = $invoker->handle($request);
66+
67+
// Verify the error message response
68+
$this->assertEquals('', (string) $response->getBody());
69+
$this->assertEquals(500, $response->getStatusCode());
70+
$this->assertTrue(
71+
$response->hasHeader(FunctionWrapper::FUNCTION_STATUS_HEADER)
72+
);
73+
$this->assertEquals(
74+
$errorStatus,
75+
$response->getHeaderLine(FunctionWrapper::FUNCTION_STATUS_HEADER)
76+
);
77+
// Verify the log output
78+
$this->assertNotNull($message);
79+
$this->assertStringContainsString('Exception: This is an error', $message);
80+
$this->assertStringContainsString('InvokerTest.php', $message); // stack trace
81+
}
82+
83+
public function provideErrorHandling()
84+
{
85+
return [
86+
['http', 'crash'],
87+
['cloudevent', 'error', new ServerRequest(
88+
'POST',
89+
'',
90+
[],
91+
'{"eventId":"foo","eventType":"bar","resource":"baz"}'
92+
)],
93+
];
94+
}
95+
4396
public function invokeThis(ServerRequestInterface $request)
4497
{
4598
return 'Invoked!';
4699
}
100+
101+
public function invokeHttpError(ServerRequestInterface $request)
102+
{
103+
throw new \Exception('This is an error');
104+
}
105+
106+
public function invokeCloudeventError(CloudEvent $event)
107+
{
108+
throw new \Exception('This is an error');
109+
}
47110
}

0 commit comments

Comments
 (0)