[FEATURE] Implement expected error format for JSON API [#2738269]

Problem/Motivation

We will want to implement the expected error format by JSON API. See http://jsonapi.org/format/upcoming/#errors

Proposed resolution

We should catch the exceptions and serialize the response. See for instance the deserializeBody method. We'll want to do something similar to that. Obviously we still need to have the serializer act on the exception class and create the appropiate output format.

    try {
      return $serializer->deserialize($received, DocumentWrapperInterface::class, $format, array('request_method' => $method));
    }
    catch (UnexpectedValueException $e) {
      $content = $serializer->serialize($e, $format);
      return new Response($content, 400, array('Content-Type' => $request->getMimeType($format)));
    }

Comment	File	Size	Author
#15	2738269--error-exceptions--15.patch	10.48 KB	e0ipso
#15	8.x-1.x: PHP 5.6 & MySQL 5.5, D8.2 152 pass PHP 7 & MySQL 5.5, D8.2 152 pass
#14	2738269--error-exceptions--14.patch	7.43 KB	e0ipso
#14	8.x-1.x: PHP 5.6 & MySQL 5.5, D8.2 147 pass PHP 7 & MySQL 5.5, D8.2 147 pass
#12	interdiff.txt	0 bytes	dawehner
#12	2738269-12.patch	5.65 KB	dawehner
#12	8.x-1.x: PHP 5.6 & MySQL 5.5, D8.2 146 pass PHP 7 & MySQL 5.5, D8.2 146 pass
#7	2738269-7.patch	4.51 KB	dawehner
#7	8.x-1.x: PHP 5.6 & MySQL 5.5, D8.2 102 pass PHP 7 & MySQL 5.5, D8.2 102 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

1 June 2016 at 06:55

e0ipso created an issue. See original summary.

Comment #2

gabesullice

English

CreditAttribution: gabesullice at Aten Design Group commented 10 June 2016 at 14:35

Assigned:

Unassigned

» gabesullice

Working on this today. Adding errors to the document is similar enough to adding links that it makes sense to do this to unblock pagination.

Comment #3

e0ipso

Catalan

Can Picafort

CreditAttribution: e0ipso as a volunteer and at Lullabot commented 18 June 2016 at 07:37

Assigned:

gabesullice

» Unassigned

Stealing this from @gabesullice. I am pretty sure that he was working on #2742769: [FEATURE] Add support for sort.

Comment #4

dawehner

German

CreditAttribution: dawehner as a volunteer commented 18 June 2016 at 09:01

Assigned:

Unassigned

» dawehner

I'll give it a try

Comment #5

e0ipso

Catalan

Can Picafort

CreditAttribution: e0ipso as a volunteer and at Lullabot commented 18 June 2016 at 16:57

My thoughts on this were something along the lines of a Normalizer for HttpException. So we can catch and serialize in the RequestHandler. But this are just initial thoughts & brainstorming.

Comment #6

dawehner

German

CreditAttribution: dawehner as a volunteer commented 18 June 2016 at 19:22

Assigned:

dawehner

» Unassigned

I rather enjoyed the weekend, so I should not try to promise anything, but I have a bit of a flight in front of me.

Having a dedicated serialization for exceptions could be really valuable. Let's hope the exceptions are semantic enough that we can actually extract the information out from there.

Comment #7

dawehner

German

CreditAttribution: dawehner as a volunteer and at Chapter Three commented 22 June 2016 at 08:48

Status:

Active

» Needs review

File	Size
2738269-7.patch	4.51 KB
8.x-1.x: PHP 5.6 & MySQL 5.5, D8.2 102 pass PHP 7 & MySQL 5.5, D8.2 102 pass

Just an idea I had in the meantime. Allow exceptions to define all those properties on itself

Comment #8

e0ipso

Catalan

Can Picafort

CreditAttribution: e0ipso as a volunteer and at Lullabot commented 20 June 2016 at 19:14

Maybe we want to leverage the extra info already compilated in https://github.com/RESTful-Drupal/restful/blob/7.x-2.x/src/Exception/Res...

Comment #9

dawehner

German

CreditAttribution: dawehner as a volunteer and at Chapter Three commented 22 June 2016 at 08:48

That could be absolutely a helpful exception class. So yeah my plan was kinda to throw exceptions as we go and then convert them and leverage it in the HTTP response. Do you think that makes sense in general?

Comment #10

gabesullice

English

CreditAttribution: gabesullice at Aten Design Group commented 24 June 2016 at 17:05

+++ b/src/Exception/JsonApiExceptionTrait.php
@@ -0,0 +1,142 @@
+trait JsonApiExceptionTrait {

Nice!

+++ b/src/Exception/JsonapiExceptionInterface.php
@@ -0,0 +1,21 @@
+interface JsonapiExceptionInterface {

Should this be JsonApiExceptionInterface? Capital "A".

+++ b/src/RequestHandler.php
@@ -73,7 +74,35 @@ class RequestHandler extends RestRequestHandler {
-      $error['error'] = $e->getMessage();
+      $error['error'] = [
+        'detail' => $e->getMessage(),
+        'status' => $e->getStatusCode(),
+      ];
+
+      if ($e instanceof JsonapiExceptionInterface) {
+        if ($e->getId()) {
+          $error['error']['id'] = $e->getId();
+        }
+        if ($e->getLinks()) {
+          $error['error']['links'] = $e->getLinks();
+        }
+        if ($e->getCode()) {
+          $error['error']['code'] = $e->getCode();
+        }
+        if ($e->getTitle()) {
+          $error['error']['title'] = $e->getTitle();
+        }
+        if ($e->getDetail()) {
+          $error['error']['detail'] = $e->getDetail();
+        }
+        if ($e->getSource()) {
+          $error['error']['source'] = $e->getSource();
+        }
+        if ($e->getMeta()) {
+          $error['error']['meta'] = $e->getMeta();
+        }
+      }
+
       $content = $serializer->serialize($error, $format);

Perhaps JsonApiException should simply implement NormalizerInterface so we don't need all this implementation detail coded into RequestHandler?

Comment #11

gabesullice

English

CreditAttribution: gabesullice at Aten Design Group commented 23 June 2016 at 23:06

Status:

Needs review

» Needs work

Comment #12

dawehner

German

CreditAttribution: dawehner as a volunteer and at Chapter Three commented 25 June 2016 at 10:18

Status:

Needs work

» Needs review

File	Size
2738269-12.patch	5.65 KB
8.x-1.x: PHP 5.6 & MySQL 5.5, D8.2 146 pass PHP 7 & MySQL 5.5, D8.2 146 pass
interdiff.txt	0 bytes

@gabesullice
That is a good idea.

This is just continues work, nowhere needs perfect/comittable.

Comment #13

e0ipso

Catalan

Can Picafort

CreditAttribution: e0ipso as a volunteer and at Lullabot commented 1 July 2016 at 06:12

Assigned:

Unassigned

» e0ipso

working on this.

Comment #14

e0ipso

Catalan

Can Picafort

CreditAttribution: e0ipso as a volunteer and at Lullabot commented 1 July 2016 at 11:11

File	Size
2738269--error-exceptions--14.patch	7.43 KB
8.x-1.x: PHP 5.6 & MySQL 5.5, D8.2 147 pass PHP 7 & MySQL 5.5, D8.2 147 pass

3 files were hidden/shown/deleted

File	Size
2738269-7.patch	4.51 KB
8.x-1.x: PHP 5.6 & MySQL 5.5, D8.2 102 pass PHP 7 & MySQL 5.5, D8.2 102 pass
2738269-12.patch	5.65 KB
8.x-1.x: PHP 5.6 & MySQL 5.5, D8.2 146 pass PHP 7 & MySQL 5.5, D8.2 146 pass
interdiff.txt	0 bytes

Feature implemented and tests added.

I added a bit of extra information if the user can see the reports.

Comment #15

e0ipso

Catalan

Can Picafort

CreditAttribution: e0ipso as a volunteer and at Lullabot commented 3 July 2016 at 07:42

File	Size
2738269--error-exceptions--15.patch	10.48 KB
8.x-1.x: PHP 5.6 & MySQL 5.5, D8.2 152 pass PHP 7 & MySQL 5.5, D8.2 152 pass

This will process PHP errors and warnings. I created a follow up to add the notices and warnings to the normal payload in the response, see #2759811: [FEATURE] Add notices and warnings to the normal response.

Comment #16

4 July 2016 at 10:05

dawehner committed 2586eb4 on 8.x-1.x authored by e0ipso

Issue #2738269 by dawehner, e0ipso: [FEATURE] Implement expected error...

Comment #17

dawehner

German

CreditAttribution: dawehner as a volunteer and at Chapter Three commented 4 July 2016 at 10:05

Status:

Needs review

» Active

IMHO having the more verbose interface would be valueable, but sure, we can do that later.

Comment #18

e0ipso

Catalan

Can Picafort

CreditAttribution: e0ipso as a volunteer and at Lullabot commented 4 July 2016 at 13:24

@dawehner Agreee. I only reverted it because it would require to track the existing HttpExceptions and have them decorated with JsonApiException. My intention was to add a follow up, but I forgot. :-P