Bug 28020

Summary: Error responses should return a code
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: RESOLVED FIXED QA Contact: Jonathan Druart <jonathan.druart+koha>
Severity: enhancement    
Priority: P5 - low CC: dcook, jonathan.druart+koha, kyle, martin.renvoize
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This development makes our error responses include an `error_code` that will be documented on each route, allowing better API usage on error conditions.
Version(s) released in:
22.05.00
Bug Depends on: 29018, 29620    
Bug Blocks: 29772    
Attachments: Bug 28020: Unit tests
Bug 28020: Add error_code to error responses
Bug 28020: (follow-up) Add docs about error_code on the API
Bug 28020: Unit tests
Bug 28020: Add error_code to error responses
Bug 28020: (follow-up) Add docs about error_code on the API
Bug 28020: Unit tests
Bug 28020: Add error_code to error responses
Bug 28020: (follow-up) Add docs about error_code on the API
Bug 28020: Unit tests
Bug 28020: Add error_code to error responses
Bug 28020: (follow-up) Add docs about error_code on the API

Description Tomás Cohen Arazi 2021-03-22 19:52:21 UTC
Right now, many error conditions on the REST API correctly return an error status (e.g. 500, 400, 403, etc) and a nice description of the error (e.g. "Anonymous patron cannot be deleted").

But, in order to ease building translatable UIs based on the REST API, we need to add an 'error_code' attribute, and probably a payload to the responses for using later to query a key/value collection (or similar).
Comment 1 David Cook 2021-05-11 00:13:40 UTC
+1
Comment 2 Tomás Cohen Arazi 2021-12-27 11:33:51 UTC
Created attachment 128915 [details] [review]
Bug 28020: Unit tests

The unhandled_exception() Mojo helper didn't have proper tests. As this
bug is adding a slight behavior change, I tried to add some. As it
relies on the OpenAPI plugin, it cannot be done the usual way using
Mojo::Lite. So I picked a route and forced an exception through mocking
to be able to write tests.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/unhandled_exceptions.t
=> FAIL: The unhandled_exception() helper doesn't return an error_code
in the response payload.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2021-12-27 11:33:55 UTC
Created attachment 128916 [details] [review]
Bug 28020: Add error_code to error responses

This patch adds the *error_code* attribute to the error response
definition. This way it can be added when returning an error.

I also made a sample usage of it on the generic unhandled_exception
helper, which the unit tests on this patchset expect.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/unhandled_exceptions.t
=> FAIL: The unhandled_exception() helper doesn't return an error_code
in the response payload.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi 2021-12-27 13:36:36 UTC
Created attachment 128917 [details] [review]
Bug 28020: (follow-up) Add docs about error_code on the API

This patch intends to be a guide for inserting Markdown documentation
for error codes. The idea is that it can be copied and pasted as-is in
new routes. And adapted to new error codes.

To test:
1. Apply this patch
2. Run:
   $ npx redoc-cli@0.10.4 bundle --cdn --output index.html \
            api/v1/swagger/swagger.yaml
=> SUCCESS: It builds correctly
3. Open index.tml on your browser
4. Pick a route, and see the 500 status description includes information
   about the possible `error_code` values.
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Andrew Fuerste-Henry 2021-12-27 14:10:49 UTC
Created attachment 128918 [details] [review]
Bug 28020: Unit tests

The unhandled_exception() Mojo helper didn't have proper tests. As this
bug is adding a slight behavior change, I tried to add some. As it
relies on the OpenAPI plugin, it cannot be done the usual way using
Mojo::Lite. So I picked a route and forced an exception through mocking
to be able to write tests.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/unhandled_exceptions.t
=> FAIL: The unhandled_exception() helper doesn't return an error_code
in the response payload.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 6 Andrew Fuerste-Henry 2021-12-27 14:10:53 UTC
Created attachment 128919 [details] [review]
Bug 28020: Add error_code to error responses

This patch adds the *error_code* attribute to the error response
definition. This way it can be added when returning an error.

I also made a sample usage of it on the generic unhandled_exception
helper, which the unit tests on this patchset expect.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/unhandled_exceptions.t
=> FAIL: The unhandled_exception() helper doesn't return an error_code
in the response payload.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 7 Andrew Fuerste-Henry 2021-12-27 14:10:56 UTC
Created attachment 128920 [details] [review]
Bug 28020: (follow-up) Add docs about error_code on the API

This patch intends to be a guide for inserting Markdown documentation
for error codes. The idea is that it can be copied and pasted as-is in
new routes. And adapted to new error codes.

To test:
1. Apply this patch
2. Run:
   $ npx redoc-cli@0.10.4 bundle --cdn --output index.html \
            api/v1/swagger/swagger.yaml
=> SUCCESS: It builds correctly
3. Open index.tml on your browser
4. Pick a route, and see the 500 status description includes information
   about the possible `error_code` values.
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 8 Tomás Cohen Arazi 2021-12-27 14:15:35 UTC
Rebasing on top of bug 29018 as that one needs to be backportable.
Comment 9 Tomás Cohen Arazi 2021-12-27 14:18:49 UTC
Created attachment 128921 [details] [review]
Bug 28020: Unit tests

The unhandled_exception() Mojo helper didn't have proper tests. As this
bug is adding a slight behavior change, I tried to add some. As it
relies on the OpenAPI plugin, it cannot be done the usual way using
Mojo::Lite. So I picked a route and forced an exception through mocking
to be able to write tests.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/unhandled_exceptions.t
=> FAIL: The unhandled_exception() helper doesn't return an error_code
in the response payload.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 10 Tomás Cohen Arazi 2021-12-27 14:18:53 UTC
Created attachment 128922 [details] [review]
Bug 28020: Add error_code to error responses

This patch adds the *error_code* attribute to the error response
definition. This way it can be added when returning an error.

I also made a sample usage of it on the generic unhandled_exception
helper, which the unit tests on this patchset expect.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/unhandled_exceptions.t
=> FAIL: The unhandled_exception() helper doesn't return an error_code
in the response payload.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 11 Tomás Cohen Arazi 2021-12-27 14:18:58 UTC
Created attachment 128923 [details] [review]
Bug 28020: (follow-up) Add docs about error_code on the API

This patch intends to be a guide for inserting Markdown documentation
for error codes. The idea is that it can be copied and pasted as-is in
new routes. And adapted to new error codes.

To test:
1. Apply this patch
2. Run:
   $ npx redoc-cli@0.10.4 bundle --cdn --output index.html \
            api/v1/swagger/swagger.yaml
=> SUCCESS: It builds correctly
3. Open index.tml on your browser
4. Pick a route, and see the 500 status description includes information
   about the possible `error_code` values.
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 12 Jonathan Druart 2022-01-03 13:39:11 UTC
Created attachment 128961 [details] [review]
Bug 28020: Unit tests

The unhandled_exception() Mojo helper didn't have proper tests. As this
bug is adding a slight behavior change, I tried to add some. As it
relies on the OpenAPI plugin, it cannot be done the usual way using
Mojo::Lite. So I picked a route and forced an exception through mocking
to be able to write tests.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/unhandled_exceptions.t
=> FAIL: The unhandled_exception() helper doesn't return an error_code
in the response payload.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2022-01-03 13:39:15 UTC
Created attachment 128962 [details] [review]
Bug 28020: Add error_code to error responses

This patch adds the *error_code* attribute to the error response
definition. This way it can be added when returning an error.

I also made a sample usage of it on the generic unhandled_exception
helper, which the unit tests on this patchset expect.

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/unhandled_exceptions.t
=> FAIL: The unhandled_exception() helper doesn't return an error_code
in the response payload.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Jonathan Druart 2022-01-03 13:39:20 UTC
Created attachment 128963 [details] [review]
Bug 28020: (follow-up) Add docs about error_code on the API

This patch intends to be a guide for inserting Markdown documentation
for error codes. The idea is that it can be copied and pasted as-is in
new routes. And adapted to new error codes.

To test:
1. Apply this patch
2. Run:
   $ npx redoc-cli@0.10.4 bundle --cdn --output index.html \
            api/v1/swagger/swagger.yaml
=> SUCCESS: It builds correctly
3. Open index.tml on your browser
4. Pick a route, and see the 500 status description includes information
   about the possible `error_code` values.
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Fridolin Somers 2022-01-05 23:35:10 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄