Skip to content

[5.4] Adjust status code of com_joomlaupdate APIs if automated updates are disabled #45966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 23, 2025

Conversation

SniperSister
Copy link
Contributor

Pull Request for Issue #45962

Summary of Changes

Throw a 404 instead of a 500

Testing Instructions

  • Install 5.4.0-beta1 on public available site
  • Make sure that autoupdates are disabled
  • Using a http client of choice, perform a request against /api/index.php/v1/joomlaupdate/healthcheck

Actual result BEFORE applying this Pull Request

500 thrown

Expected result AFTER applying this Pull Request

404 thrown

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@HLeithner
Copy link
Member

HLeithner commented Aug 22, 2025

409 would fit better I think

@SniperSister
Copy link
Contributor Author

... and I think that the general error handling of the JsonapiRenderer needs a serious refactoring as it's currently impossible to define an status code from an application level, you have to rely on the pre-defined handlers and their hardcoded codes.

@richard67
Copy link
Member

Hmm, 409 indeed seems more suitable than 404.

@SniperSister
Copy link
Contributor Author

As I said: the API does not allow custom status codes. Specific exceptions are associated with specific codes. We would have to add a new exception type for a 409, which just builds yet another layer on this insufficient base design. I’m not doing this in my PR. Either use it or propose an alternate approach.

@richard67
Copy link
Member

I'm ok with that. As the Rolling Stones sang: You can't always get what you want :-)

@cybersalt
Copy link

I have tested this item ✅ successfully on 0cd8e66

Received a 404 after the patch.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45966.

@exlemor
Copy link

exlemor commented Aug 23, 2025

I have tested this item ✅ successfully on 0cd8e66

I have tested this successfully with Tim ;) Thanks @SniperSister!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45966.

@rbuelund
Copy link

I have tested this item ✅ successfully on 0cd8e66

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45966.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 23, 2025
@muhme
Copy link
Contributor

muhme commented Aug 23, 2025

✅ Final test before merge, using public instance with manual installation

  • before the PR
curl -H 'X-JUpdate-Token: 4711' 'https://joomla-test.heikol.de/api/index.php/v1/joomlaupdate/healthcheck'
{"errors":[{"code":500,"title":"Internal server error"}]}%  
  • After second installation with this PR from full package:
    {"errors":[{"title":"Resource not found","code":404}]}%

@muhme muhme merged commit 029040c into joomla:5.4-dev Aug 23, 2025
40 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 23, 2025
@muhme muhme added this to the Joomla! 5.4.0 milestone Aug 23, 2025
@muhme muhme removed the PBF Pizza, Bugs and Fun label Aug 23, 2025
@muhme
Copy link
Contributor

muhme commented Aug 23, 2025

Thank you @SniperSister for your contribution. Thank you @HLeithner and @richard67 for supporting this issue. Thank you @rbuelund and @exlemor for testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants