home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.67k stars 30.8k forks source link

Synology integration failing to initiate due to failing API. #126921

Closed ab623 closed 1 month ago

ab623 commented 1 month ago

The problem

When trying to add Synology DSM as a new integration into Home Assistant, it fails to set up.

Having done some investigation, I believe it is because a single Synology API failing to return, specific the SecurityScan API. This causes the integration to fail and not set up any entities at all. I have attached the appropriate logs that show this.

For my purposes, I do not need to Security Scan API sensors, because that will just give me general update information. I would rather:

What version of Home Assistant Core has the issue?

core-2024.9.3

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

Synology DSM

Link to integration documentation on our website

https://www.home-assistant.io/integrations/synology_dsm/

Diagnostics information

home-assistant_synology_dsm_2024-09-27T13-03-04.684Z.log

Example YAML snippet

No response

Anything in the logs that might be useful for us?

2024-09-27 14:01:42.275 DEBUG (MainThread) [synology_dsm.synology_dsm] Request url: http://192.168.1.200:5000/webapi/entry.cgi?api=SYNO.Core.SecurityScan.Status&version=1&method=system_get&_sid=*********
2024-09-27 14:01:42.276 DEBUG (MainThread) [synology_dsm.synology_dsm] Response status_code: 200
2024-09-27 14:01:42.276 DEBUG (MainThread) [synology_dsm.synology_dsm] Response headers: {'Server': 'nginx', 'Date': 'Fri, 27 Sep 2024 13:01:42 GMT', 'Content-Type': 'application/json; charset="UTF-8"', 'Transfer-Encoding': 'chunked', 'Connection': 'keep-alive', 'Keep-Alive': 'timeout=20', 'Vary': 'Accept-Encoding', 'X-Content-Type-Options': 'nosniff', 'X-XSS-Protection': '1; mode=block', 'Cache-Control': 'max-age=0, no-cache, no-store, must-revalidate', 'Pragma': 'no-cache', 'Expires': '0', 'Content-Encoding': 'gzip'}
2024-09-27 14:01:42.276 DEBUG (MainThread) [synology_dsm.synology_dsm] Successful returned data
2024-09-27 14:01:42.276 DEBUG (MainThread) [synology_dsm.synology_dsm] RESPONSE: {'error': {'code': 1300}, 'success': False}
2024-09-27 14:01:42.276 DEBUG (MainThread) [synology_dsm.synology_dsm] Session error: 1300
2024-09-27 14:01:42.276 DEBUG (MainThread) [homeassistant.components.synology_dsm.common] Connection error during setup of '2050SBRVJPF5Y' with exception: {'api': 'SYNO.Core.SecurityScan.Status', 'code': 1300, 'reason': 'Unknown', 'details': None}

Additional information

My Synology device does not open Security Advisor in the front end. It fails with a generic "Operation Failed" error. Having inspected the front end during this error, it makes 100's of calls to the same API with the same error as HomeAssistant.

The payload: api=SYNO.Core.SecurityScan.Status&method=system_get&version=1

The response is always: {"error":{"code":1300},"success":false}

This is deemed to be unfixable by technology support because I have "custom packages" installed on my system and they are unable to investigate further.

home-assistant[bot] commented 1 month ago

Hey there @hacf-fr, @quentame, @mib1185, mind taking a look at this issue as it has been labeled with an integration (synology_dsm) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `synology_dsm` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign synology_dsm` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


synology_dsm documentation synology_dsm source (message by IssueLinks)

mib1185 commented 1 month ago

My Synology device does not open Security Advisor in the front end. It fails with a generic "Operation Failed" error. Having inspected the front end during this error, it makes 100's of calls to the same API with the same error as HomeAssistant.

This is deemed to be unfixable by technology support because I have "custom packages" installed on my system and they are unable to investigate further.

Based on this, it looks like an issue with your nas itself, which should be solved.

ab623 commented 1 month ago

I agree. The problem is in the device for sure. Synology support can't fix this because my system has 3rd party packages installed on it (much like everyone else's).

The ideal scenario would be the issue on the NAS is fixed but it seems like without a mode 2 reset the issue cannot be fixed. Which means I essentially need to setup my NAS all over again.

Obviously this isn't ideal, but the issue is that the HASS integration fails to set up any entity if any API fails.

My suggestion was that the HASS integration skips the entities that fails but continues on. This would enable me to continue using all the other sensors but skip the failing ones (in my case the update sensor)

Would that be a sensible feature request? As right now the entire integration is unable for me.

mib1185 commented 1 month ago

Would that be a sensible feature request?

I would consider it as such, since the issue is not caused by a deactivated feature on the nas, but by an issue with a feature. We've already some extra checks during the setup implemented in the integration for the surveillance station (missing user permissions) and upgrade checks (no internet connection), but these checks do not cover malfunction of the nas. Further I'm also struggling to implement a workaround in the integration to cover any possible malfunction in the nas - the malfunction should be resolved, since we cannot know, which further effects these malfunctions can have.

btw. the security scan is not only providing information about pending updates, it also checks your nas configuration for any security relevant configurations and provide hints about non-secure configurations.

ab623 commented 1 month ago

Hi @mib1185. Thanks for this. Is this something you are actively working on?

An alternative approach could be proving a list of Synology integrations on the setup which you don't want to be included in the integration?

mib1185 commented 1 month ago

I'm one of the integration owners (see https://github.com/home-assistant/core/issues/126921#issuecomment-2379298439) 😉

An alternative approach could be proving a list of Synology integrations on the setup which you don't want to be included in the integration?

Even this would only be a workaround to deal with issues caused by a malfunction of the nas. But we can't and won't forsee all possible malfunctions, those we should not implement such workarounds at all. The only correct solution is to solve the malfunction of the nas.

ab623 commented 1 month ago

Yes aware you are the integration owner, hence was asking if handling malfunctioning API's was a feature you were actively working on in this integration.

Unfortunately, resolving the NAS issue requires a full configuration wipe, which is too costly for me. Especially for a feature in Synology that I make no use of.

Are you then suggesting that to resolve this issue the only options I have here are:

  1. Not use the integration and look for another approach.
  2. Fork a local copy of the integration with my fixes applied

I'm a bit confused as to if this fix is something that you would consider in this integration or not?

mib1185 commented 1 month ago

was asking if handling malfunctioning API's was a feature you were actively working on in this integration.

ah now i get it, but unfortunately I've to answer "no" - as i mentioned above, we should not implement workarounds for possible device malfunctions, since we can not predict, how a malfunction will effect the device at all, so every try to "patch" such a malfunction might cause other issues (up to a non-functioning integration at all, which leads to other issue reports). So the only proper solution should be fixing the malfunction.

ab623 commented 1 month ago

Okay understood.

For anyone else interested - I "fixed" this for my usecase by making a copy of the component, and putting it in my config/custom_component folder.

I modified the manifest.json and put a version in it, so that it shows up as a custom component.

I then updated the following line https://github.com/home-assistant/core/blob/6ee03460d6e67b9eba126141de65fca76b742275/homeassistant/components/synology_dsm/__init__.py#L65

to

    api = SynoApi(hass, entry)
    api._with_security = False

This enabled setup to continue without calling any of the failing security api's.

I wrapped this up in shell script which means I can pull any of the latest updates to the real component and make my modifications. A redacted version is below without all the error checking and cleanup. I hope someone finds it useful.

#!/bin/bash

# Source location
repo_url="https://github.com/home-assistant/core"
repo_branch="dev"
repo_folder="homeassistant/components/synology_dsm"
source_temp_target="/tmp/synology_dsm_temp"

# Replace with your desired target directory
components_dir="/config/custom_components"
target_dir="$components_dir/synology_dsm"

# Clone the Git repository
git clone --depth 1 --filter=blob:none --sparse $repo_url $source_temp_target > /dev/null 2>&1
cd $source_temp_target
git sparse-checkout add $repo_folder > /dev/null 2>&1

# Move the desired folder to the target directory
mv "$source_temp_target/$repo_folder" "$components_dir/"

# Update the manifest.json
jq '. | .version = "1.0.0"' "$target_dir/manifest.json" > "$target_dir/manifest.json.tmp"
mv "$target_dir/manifest.json.tmp" "$target_dir/manifest.json"

# Update the __init__.py
sed -i '/api = SynoApi/a\ \ \ \ api._with_security = False' "$target_dir/__init__.py"

It works for me... @mib1185 can you tell me if i'm missing any glaring issues! I'm a newbie to HASS.

mib1185 commented 1 month ago

@mib1185 can you tell me if i'm missing any glaring issues! I'm a newbie to HASS.

I strongly do NOT recommend doing this (reasons mentioned above)! Please do not assume the synology_dsm code will not change in future - it might happen, that structures in HA core will change, those also for the integrations code or even just because of further developments and improvements - that said, your workaround may break in future releases and may cause different/additional issues.

ab623 commented 1 month ago

I completely agree with you. I appreciate the fact that the integration might update, HA core might update, along with any number of components may update.

In that case until I fix my synology devise, I am left with literally no other alternative but to fix it in the code via a custom component. Hence when my approach breaks, I'll just need to adjust things to fix it.