API returns HTTP 500 instead of 404 on not found #3841

New Issue

OVERLORD · 2026-02-05T07:38:45+03:00

OVERLORD commented

2026-02-05 07:38:45 +03:00

Originally created by @devdot on GitHub (Jun 8, 2023).

Describe the Bug

When requesting a non-existing page, the API returns HTTP 500 instead of 404. The JSON looks like this:

{"error":{"message":"Page not found","code":500}}

When I read the documentation (https://demo.bookstackapp.com/api/docs#error-handling) I assumed it meant that the API returns matching HTTP statuses when possible.

After clarifying on discord (thanks @ssddanbrown !), this behavior is not expected.

Steps to Reproduce

Make an authorized API call to a non-existing page (example: https://demo.bookstackapp.com/api/pages/5000)
Receive HTTP 500 instead of 404

Expected Behaviour

HTTP 404 Not Found

{"error":{"message":"Page not found","code":404}}

Screenshots or Additional Context

No response

Browser Details

No response

Exact BookStack Version

v23.05.2

PHP Version

No response

Hosting Environment

ubuntu-22.04

Originally created by @devdot on GitHub (Jun 8, 2023). ### Describe the Bug When requesting a non-existing page, the API returns HTTP 500 instead of 404. The JSON looks like this: ```json {"error":{"message":"Page not found","code":500}} ``` When I read the documentation (https://demo.bookstackapp.com/api/docs#error-handling) I assumed it meant that the API returns matching HTTP statuses when possible. After clarifying on discord (thanks @ssddanbrown !), this behavior is not expected. ### Steps to Reproduce 1. Make an authorized API call to a non-existing page (example: https://demo.bookstackapp.com/api/pages/5000) 2. Receive HTTP 500 instead of 404 ### Expected Behaviour HTTP 404 Not Found ```json {"error":{"message":"Page not found","code":404}} ``` ### Screenshots or Additional Context _No response_ ### Browser Details _No response_ ### Exact BookStack Version v23.05.2 ### PHP Version _No response_ ### Hosting Environment ubuntu-22.04

OVERLORD added the 🐛 Bug label 2026-02-05 07:38:45 +03:00

OVERLORD closed this issue

2026-02-05 07:38:47 +03:00

OVERLORD commented

2026-02-05 07:38:55 +03:00

@devdot commented on GitHub (Jun 8, 2023):

I've implemented a test to verify the issue: ecf99fa0ed

Looking into the issue, there are two relevant places:

Exceptions\NotFoundException

this exception is triggered correctly
code 404 is set correctly
the method getStatus does not exist (would be called by the exception handler, see below)
possible solution 1: implement getStatus

Exceptions\Handler::renderApiException

the default $code is 500 and Exception->getCode() only called on specific exceptions (not Exceptions\NotFoundException)
Exception->getStatus is called when it exists, overwriting $code
$code used to be set by Exception->getCode, but was changed: b4fa82e329
possible solution 2: set $code from Exception->getCode by default

Solution 2 seems to be more generic and elegant, therefore I'll submit a PR

@devdot commented on GitHub (Jun 8, 2023): I've implemented a test to verify the issue: https://github.com/BookStackApp/BookStack/commit/ecf99fa0edbddc5c47eb94b9cb3b5f22c65890ee Looking into the issue, there are two relevant places: [Exceptions\NotFoundException](https://github.com/BookStackApp/BookStack/blob/development/app/Exceptions/NotFoundException.php) - this exception is triggered correctly - code 404 is set correctly - the method `getStatus` does not exist (would be called by the exception handler, see below) - possible solution 1: implement `getStatus` [Exceptions\Handler::renderApiException](https://github.com/BookStackApp/BookStack/blob/development/app/Exceptions/Handler.php#L80) - the default `$code` is 500 and Exception->getCode() only called on specific exceptions (not Exceptions\NotFoundException) - Exception->getStatus is called when it exists, overwriting `$code` - `$code` used to be set by Exception->getCode, but was changed: https://github.com/BookStackApp/BookStack/commit/b4fa82e3298a15443ca40bff205b7a16a1031d92 - possible solution 2: set `$code` from Exception->getCode by default Solution 2 seems to be more generic and elegant, therefore I'll submit a PR

OVERLORD commented

2026-02-05 07:38:58 +03:00

@ssddanbrown commented on GitHub (Jun 13, 2023):

Thank you for much for the PR @devdot, it looks great.

The only thing I'd be nervous about is essentially always using the exception code as a status code by default.
We can't be sure what exceptions might go through this, and I don't think we can always assure they'd be using http aligned codes.

Could I propose a tweak on your solution 1 above?
The NotFoundException extends PrettyException. Looking at PrettyException, that specific exception class does expect the code to be a http status code (where not zero otherwise default to 500).
getStatus could instead be added to PrettyException to generically handle these types of exceptions across the API, without assuming codes as statuses for all exceptions.

Optionally, If you wanted to go a step further, you could look to clean up some of the messiness/duplication I've created here over the years. There's the getStatus method check & use, which is pretty hacky, but there's already this:

5e8ec56196/app/Exceptions/Handler.php (L85-L88)

Instead of checking against HttpException here, we could check for an instance of HttpExceptionInterface, then implement that interface within PrettyException. Other exceptions making use of getStatus here could be updated to use HttpExceptionInterface also, so then getStatus could be removed. That way, we've sticking to one main method of setting http status in exceptions, in a manner working to interfaces instead of my hacky method checks.

@ssddanbrown commented on GitHub (Jun 13, 2023): Thank you for much for the PR @devdot, it looks great. The only thing I'd be nervous about is essentially always using the exception code as a status code by default. We can't be sure what exceptions might go through this, and I don't think we can always assure they'd be using http aligned codes. Could I propose a tweak on your solution 1 above? The `NotFoundException` extends `PrettyException`. Looking at `PrettyException`, that specific exception class does expect the code to be a http status code (where not zero otherwise default to 500). `getStatus` could instead be added to `PrettyException` to generically handle these types of exceptions across the API, without assuming codes as statuses for all exceptions. Optionally, If you wanted to go a step further, you could look to clean up some of the messiness/duplication I've created here over the years. There's the `getStatus` method check & use, which is pretty hacky, but there's already this: https://github.com/BookStackApp/BookStack/blob/5e8ec56196f94a86b9600a2dec031ec92afc0170/app/Exceptions/Handler.php#L85-L88 Instead of checking against `HttpException` here, we could check for an instance of `HttpExceptionInterface`, then implement that interface within `PrettyException`. Other exceptions making use of `getStatus` here could be updated to use `HttpExceptionInterface` also, so then `getStatus` could be removed. That way, we've sticking to one main method of setting http status in exceptions, in a manner working to interfaces instead of my hacky method checks.

OVERLORD commented

2026-02-05 07:38:59 +03:00

@devdot commented on GitHub (Jun 13, 2023):

Hi @ssddanbrown, I agree, using the exception code as HTTP status could go wrong in unexpected ways. I was actually quite surprised to see that it used to be like that (as I referenced above) and that all tests still go green after I reverted back to always using getCode.

I think using HttpExceptionInterface is the best option. Yes, I found it a bit messy to figure out whats going on in the exception handler, so a clean up as you described would be good. I'll update the PR in the next few days

@devdot commented on GitHub (Jun 13, 2023): Hi @ssddanbrown, I agree, using the exception code as HTTP status could go wrong in unexpected ways. I was actually quite surprised to see that it used to be like that (as I referenced above) and that all tests still go green after I reverted back to always using `getCode`. I think using `HttpExceptionInterface` is the best option. Yes, I found it a bit messy to figure out whats going on in the exception handler, so a clean up as you described would be good. I'll update the PR in the next few days

OVERLORD commented

2026-02-05 07:39:02 +03:00

@devdot commented on GitHub (Jun 15, 2023):

This issue should be done with #4291 , unless you'd want to go forward with a bigger refactoring of exceptions as I've outlined in #4309

@devdot commented on GitHub (Jun 15, 2023): This issue should be done with #4291 , unless you'd want to go forward with a bigger refactoring of exceptions as I've outlined in #4309

OVERLORD commented

2026-02-05 07:39:05 +03:00

@ssddanbrown commented on GitHub (Jun 15, 2023):

Addressed in #4291, to be part of next feature release.

@ssddanbrown commented on GitHub (Jun 15, 2023): Addressed in #4291, to be part of next feature release.

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/BookStack#3841