Bug 28020 - Error responses should return a code
Summary: Error responses should return a code
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 29018 29620
Blocks: 29772
  Show dependency treegraph
 
Reported: 2021-03-22 19:52 UTC by Tomás Cohen Arazi
Modified: 2022-01-07 11:27 UTC (History)
4 users (show)

See Also:
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


Attachments
Bug 28020: Unit tests (3.22 KB, patch)
2021-12-27 11:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28020: Add error_code to error responses (1.90 KB, patch)
2021-12-27 11:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28020: (follow-up) Add docs about error_code on the API (44.52 KB, patch)
2021-12-27 13:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28020: Unit tests (3.28 KB, patch)
2021-12-27 14:10 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28020: Add error_code to error responses (1.97 KB, patch)
2021-12-27 14:10 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28020: (follow-up) Add docs about error_code on the API (44.59 KB, patch)
2021-12-27 14:10 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 28020: Unit tests (3.28 KB, patch)
2021-12-27 14:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28020: Add error_code to error responses (1.97 KB, patch)
2021-12-27 14:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28020: (follow-up) Add docs about error_code on the API (44.54 KB, patch)
2021-12-27 14:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28020: Unit tests (3.36 KB, patch)
2022-01-03 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28020: Add error_code to error responses (2.04 KB, patch)
2022-01-03 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28020: (follow-up) Add docs about error_code on the API (44.67 KB, patch)
2022-01-03 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 🦄