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

fix: delete cookiefile when removing config entry #2462

Merged
merged 5 commits into from
Aug 18, 2024

Conversation

danielbrunt57
Copy link
Collaborator

Delete pickle file when removing config entry as it's no longer needed and can cause issues when re-adding the integration entry.

danielbrunt57 and others added 2 commits August 16, 2024 19:37
Added code to delete pickle file when removing config entry.
@alandtse
Copy link
Owner

Can you confirm the file isn't handled at the alexapy level? If so, any deletion should be handled there via an API call.

danielbrunt57 and others added 2 commits August 16, 2024 19:57
Fixed an error in _LOGGER when file can't be deleted
@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Aug 17, 2024

No, it's not and it always gets left behind when the config entry is removed and has caused issues galore for a little while now for users trying to fix other setup issues. Countless posts stating "Delete the integration, delete the pickle file and try again!"
I added it to async def async_unload_entry (line 1289) where everything but it gets removed when the config entry is deleted and felt that was the appropriate place.
Please review and advise...

@danielbrunt57
Copy link
Collaborator Author

The only reference in alexapy to the pickle file is in alexalogin.py:

def __init__(

        self._cookiefile: list[str] = [
            self._outputpath(f".storage/{self._hass_domain}.{self.email}.pickle"),
            self._outputpath(f"{self._hass_domain}.{self.email}.pickle"),
            self._outputpath(f".storage/{self._hass_domain}.{self.email}.txt"),
        ]

and this:

    async def save_cookiefile(self) -> None:
        """Save login session cookies to file."""
        self._cookiefile = [
            self._outputpath(f".storage/{self._hass_domain}.{self.email}.pickle"),
            self._outputpath(f"{self._hass_domain}.{self.email}.pickle"),
            self._outputpath(f".storage/{self._hass_domain}.{self.email}.txt"),
        ]
        for cookiefile in self._cookiefile:
            if cookiefile == self._cookiefile[0]:
                cookie_jar = self._session.cookie_jar
                assert isinstance(cookie_jar, aiohttp.CookieJar)
                if self._debug:
                    _LOGGER.debug("Saving cookie to %s", cookiefile)
                try:
                    cookie_jar.save(self._cookiefile[0])
                except (OSError, EOFError, TypeError, AttributeError) as ex:
                    _LOGGER.debug(
                        "Error saving pickled cookie to %s: %s",
                        self._cookiefile[0],
                        EXCEPTION_TEMPLATE.format(type(ex).__name__, ex.args),
                    )
            elif (cookiefile) and os.path.exists(cookiefile):
                _LOGGER.debug("Removing outdated cookiefile %s", cookiefile)
                await delete_cookie(cookiefile)
        if self._debug:
            _LOGGER.debug("Session Cookies:\n%s", self._print_session_cookies())

@alandtse
Copy link
Owner

Then you should create a delete_cookiefile in alexapy and then call that in amp.

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Aug 17, 2024

Something like this then?

image

image

alexapy debug:

2024-08-17 16:37:38.560 DEBUG (MainThread) [alexapy.alexaapi] dl@b**a: Trying post: https://alexa.amazon.com/api/behaviors/preview : with uri: /api/behaviors/preview data {'behaviorId': 'PREVIEW', 'sequenceJson': '{"@type": "com.amazon.alexa.behaviors.model.Sequence", "startNode": {"@type": "com.amazon.alexa.behaviors.model.SerialNode", "nodesToExecute": [{"@type": "com.amazon.alexa.behaviors.model.OpaquePayloadOperationNode", "type": "Alexa.Speak", "operationPayload": {"deviceType": "A2U21SRK4QGSE1", "deviceSerialNumber": "G091AA08051216SA", "locale": "en-ca", "customerId": "A1UZ93DGNU3F5L", "textToSpeak": "Home Assistant startup is complete", "target": {"customerId": "A1UZ93DGNU3F5L", "devices": [{"deviceSerialNumber": "G091AA08051216SA", "deviceTypeId": "A2U21SRK4QGSE1"}]}, "skillId": "amzn1.ask.1p.saysomething"}}]}}', 'status': 'ENABLED'} query None

2024-08-17 16:37:38.676 DEBUG (MainThread) [alexapy.alexaapi] dl@b**a: POST: https://alexa.amazon.com/api/behaviors/preview returned 200:OK:application/octet-stream

2024-08-17 16:39:42.804 DEBUG (MainThread) [alexapy.helpers] Deleting cookiefile /config/.storage/[email protected]

alexa_media debug:

2024-08-17 16:37:37.054 DEBUG (MainThread) [custom_components.alexa_media.notify] TTS by : Home Assistant startup is complete
2024-08-17 16:39:42.745 DEBUG (MainThread) [custom_components.alexa_media] Attempting to unload entry for dl@ba
2024-08-17 16:39:42.745 DEBUG (MainThread) [custom_components.alexa_media] Forwarding unload entry to media_player
2024-08-17 16:39:42.751 DEBUG (MainThread) [custom_components.alexa_media] Forwarding unload entry to notify
2024-08-17 16:39:42.760 DEBUG (MainThread) [custom_components.alexa_media] Forwarding unload entry to switch
2024-08-17 16:39:42.776 DEBUG (MainThread) [custom_components.alexa_media] Forwarding unload entry to sensor
2024-08-17 16:39:42.788 DEBUG (MainThread) [custom_components.alexa_media] Forwarding unload entry to alarm_control_panel
2024-08-17 16:39:42.788 DEBUG (MainThread) [custom_components.alexa_media] Forwarding unload entry to light
2024-08-17 16:39:42.789 DEBUG (MainThread) [custom_components.alexa_media] Forwarding unload entry to binary_sensor
2024-08-17 16:39:42.790 DEBUG (MainThread) [custom_components.alexa_media.notify] Attempting to unload notify
2024-08-17 16:39:42.803 DEBUG (MainThread) [custom_components.alexa_media] d
l@ba: Connection closed: True
2024-08-17 16:39:42.803 DEBUG (MainThread) [custom_components.alexa_media] Removing accounts data and services
2024-08-17 16:39:42.803 DEBUG (MainThread) [custom_components.alexa_media] Removing config_flows data
2024-08-17 16:39:42.804 DEBUG (MainThread) [custom_components.alexa_media] Removing alexa_media data structure
2024-08-17 16:39:42.804 DEBUG (MainThread) [alexapy.helpers] Deleting cookiefile /config/.storage/[email protected]
2024-08-17 16:39:42.835 DEBUG (MainThread) [custom_components.alexa_media] Deleted cookiefile
2024-08-17 16:39:42.835 DEBUG (MainThread) [custom_components.alexa_media] Unloaded entry for d
l@b******a

image

Uses new alexapy function delete_cookiefile() instead of os.remove(pickle)
@danielbrunt57 danielbrunt57 changed the title feat: delete pickle file from .storage when removing config entry fix: delete cookiefile (aka pickle file) from .storage when removing config entry Aug 18, 2024
@danielbrunt57
Copy link
Collaborator Author

Note: this fix requires alexapy merge request !380

@danielbrunt57 danielbrunt57 added the alexapy Issue relates to the API label Aug 18, 2024
@alandtse
Copy link
Owner

So what was the deal before for why it was failing?

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Aug 18, 2024

It never was failing. It was never even trying!
When I first saw the alexapy helper at work, I was slightly confused/mislead by it but then I realized that my code addition in alexapy is actually using that helper, which is used elsewhere in alexapy. But there was no code in alexapy that AMP's async def async_unload_entry() could call to delete the cookiefile.
If I could call the delete_cookie() directly from AMP, then alexapy would not need to be changed, but I don't see where/how AMP knows what the cookiefile for this entry is...

@alandtse alandtse changed the title fix: delete cookiefile (aka pickle file) from .storage when removing config entry fix: delete cookiefile when removing config entry Aug 18, 2024
@alandtse alandtse merged commit 7b5524c into alandtse:dev Aug 18, 2024
4 checks passed
alandtse added a commit that referenced this pull request Aug 18, 2024
fix: delete cookiefile when removing config entry (#2462)
@danielbrunt57
Copy link
Collaborator Author

I see you've been busy! What about the underlying requirement in alexapy? v4.12.8 will need a new alexapy (==1.28.3?) to be able to delete the cookiefile...

@alandtse
Copy link
Owner

I mean if it required a version bump it should've been in the PR. Please only submit complete PRs. I don't have time to guess if it needs more parts to it.

@alandtse
Copy link
Owner

Please mark it as a Draft if it's incomplete.

@danielbrunt57
Copy link
Collaborator Author

I mean if it required a version bump it should've been in the PR.

Forgive me for still being quite new to this PR stuff and not knowing everything yet but I do not know how or where that would be specified. I did add comment Note: this fix requires alexapy merge request !380 8 hours ago but I guess you're too busy to have seen that...

@chrisvblemos
Copy link
Contributor

In these cases, I suggest you first submit the PR to AlexaPy and then only after AlexaPy has your PR merged you submit the PR here, including the bump in AlexaPy's version. Might take some time, given alandtse free time, but it is the safest way.

@danielbrunt57
Copy link
Collaborator Author

@chrisvblemos
Yes, I can see now after-the-fact that I did it incorrectly but thank you for pointing out my mistake!
Perhaps v4.12.8 can be pulled to prevent more reports of an error that have already started to appeared in AMP issues. See Voice notifications #2442 ...

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Aug 18, 2024

I've added a new issue (#2470) with how to eliminate the error for now.
I'm testing this code as a solution for the new error until the matching alexapy merge request is completed with a version bump and alexa_media is also bumped with a new alexapy version requirement in manifest.json...

        # Delete cookiefile
        if callable(getattr(AlexaLogin,'delete_cookiefile', None)):
          try:
              await login_obj.delete_cookiefile()
              _LOGGER.debug("Deleted cookiefile")
          except Exception as ex:
              _LOGGER.error(
                  "Failed to delete cookiefile: %s",
                  ex,
              )
        else:
            _LOGGER.warn(
                "Could not remove cookiefile: /config/.storage/alexa_media.%s.pickle."
                "  Please manually delete the file.",
                email,
            )

@danielbrunt57
Copy link
Collaborator Author

New PR #2471 submitted to check if AlexaLogin.delete_cookiefile exists before trying to call it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alexapy Issue relates to the API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants