Skip to content
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

API POST endpoint /rest/addons always returns 200 even with wrong parameters #3987

Closed
crazyelectron-io opened this issue Dec 31, 2023 · 4 comments · Fixed by #3989
Closed
Labels
bug An unexpected problem or unintended behavior of the Core

Comments

@crazyelectron-io
Copy link

POST-ing to /rest/addons to install a marketplace add-on will always return 200 (OK) with an empty body, no matter what parameters are passed. Also when installing an already installed add-on, it gives the same result.
I would expect a 404 if it gets passed a non-existing add-on.

This is a simple script (rule) I use to install marketplace addons during startup:

rule "InstallMarketAddons" do
  description "Install marketplace add-ons at system start"
  on_start at_level: 80
  delay 20.seconds  # Let OH core do its magic first
  run do
    # Extract the marketplace add-ons line from `conf/services/addons.cfg`
    cfgLines = IO.readlines('{{ openhab_conf_dir }}/services/addons.cfg')
    if lineIndex = cfgLines.index{ |line| line =~ /marketplace/ } then
      logger.info "Install Marketplace addons defined in addons.cfg at system start"
      addons = cfgLines[lineIndex].split('=')[1].strip
      # Loop through each addon
      addons.split(",").each do |addon|
        addonName, addonId = addon.split(":")
        #TODO: Check for ill-formatted entries
        logger.info "Install addon #{addonName} (#{addonId}) from marketplace"
        # Create the HTTP object
        uri = URI.parse("{{ openhab_api_url }}/addons/marketplace:#{addonId}/install?serviceId=marketplace")
        header = {'Content-Type': 'text/plain', 'Accept': '*/*', 'X-OPENHAB-TOKEN': '{{ openhab_api_token }}'}
        http = Net::HTTP.new(uri.host, uri.port)
        request = Net::HTTP::Post.new(uri.request_uri, header)
        request.body = "id=#{addonId}"
        # Send the request
        response = http.request(request)
        # Note: The API always returns 200 with an empty body, so actually no use logging it
        logger.debug "Result #{response.code} (#{response.body}) - installing marketplace addon #{addonName} (#{addonId})"
      end # of loop through each entry
    end # if
  end
end

There are two Ansible variables in the script to specify the base URL of the API endpoint and the token.
The script works by reading a line starting with marketplace= and extracting the add-on id's to install.
By specifying a non-exsitings binding the reported behaviour can be reproduced.

Your Environment

Debian 12.
OpenHAB 4.1.0 using the official OH container image.

@crazyelectron-io crazyelectron-io added the bug An unexpected problem or unintended behavior of the Core label Dec 31, 2023
@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/solved-install-marketplace-addon-via-rest-api-returns-200-but-nothing-is-installed/152399/7

@J-N-K
Copy link
Member

J-N-K commented Dec 31, 2023

The reason is that the installation is asynchronous. The response is create when the add-on service exists and the command to install has been successfully send. We can probably improve that with an additional check if the service supports that add-on.

@crazyelectron-io
Copy link
Author

crazyelectron-io commented Dec 31, 2023

Thanks for your quick response.
I understand that the installation is asynchronous - like just about everything in an event-driven system like OpenHBA - and there is no callback mechanism to get the actual result. So, to check if an add-on is installed would require a (delayed/retried) GET to /rest/addons/{addend}.

My main point is that it accepts non-existing add-ons and like you said that could be improved. :o)
The documentation states that a 404 can be returned, but it looks like that will never happen with the current implementation.

@crazyelectron-io
Copy link
Author

Wow, this gives "quick-fix" a whole new meaning! :o)
Reported, fixed and merged in less than 7 hours...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of the Core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants