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

Creation of dynamic property is deprecated in PHP 8.2+, which breaks shutdown handler and therefore error reporting #409

Open
SriRamanujam opened this issue Mar 31, 2024 · 6 comments
Labels
Self-Hosting Related to self-hosting beestat.

Comments

@SriRamanujam
Copy link

SriRamanujam commented Mar 31, 2024

Describe the bug
So I'm trying to stand up a self-hosted beestat, both for running it and for potentially contributing to it in the future. Thank you so much for your hard work on this project, I'm looking forward to using it :D

I attempted to stand things up in a Docker container using the php:8.3-apache image. This is how I encountered this issue. This bug report is more of a heads-up, rather than something currently broken.

The codebase makes heavy use of dynamic properties, which are deprecated in PHP 8.2+. This deprecation gets flagged as an error due to the error_reporting setting (I believe? It's been a few years since I've worked with PHP), which is fine.

But then the shutdown handler tries to set a dynamic property as well, which then completely blows up the whole thing and I get a empty page with no error data whatsoever.

Other notes:
The self-hosting docs don't make mention of which PHP version to use, it just says PHP 8. I admit to swinging for the fences a bit with PHP 8.3 since Jammy only has 8.1, but hey, nothing ventured nothing gained right?

To Reproduce

  1. Stand up the app using PHP 8.2 or newer.
  2. Browse to the homepage.
  3. Observe a blank page (empty html) with no error data returned in server logs or on the page itself.

Screenshots
Screenshot from my VS Code showing the error and trace, since there's no other way to get it

image

@ziebelje
Copy link
Member

Appreciate the detailed report! I suppose the appropriate fix is to declare these properties...probably quite a few that need done. For now I updated the documentation to specify 8.1. I try to keep things reasonably up-to-date, so I'm sure I'll get to this eventually.

Let me know if you have any other issues getting this up and running.

@patricknelson
Copy link

I've used Rector in the past and it's fantastic (https://github.com/rectorphp/rector). You should consider checking it out. You can configure it to change only exactly what you need if you want. For example, the relevant rule here I think is AddPropertyTypeDeclarationRector: https://getrector.com/rule-detail/add-property-type-declaration-rector

@patricknelson
Copy link

patricknelson commented Oct 2, 2024

p.s. On that note of getting setup with local development (potentially also for self-hosting): A lot of the instructions in self-hosting can be automated with docker-compose.yml. One potential blocker I'm identifying however is the need to integrate with Ecobee (per docs), at least presumably if you want to get real data and not demo data.

Is that still an issue as far as you know, @ziebelje? Also, can you confirm that you can at least use the demo version for local development? I was considering submitting something minor myself.

@ziebelje
Copy link
Member

Part of the setup does involve setting up an API account with ecobee (which is currently disabled). There's no way around that as you do need that in order to really do anything productive in beestat.

The demo version (demo.beestat.io or locally just setting the config to use the demo) would not work locally. That's more of a custom thing that uses special database views to show data.

@patricknelson
Copy link

I realized I sort of forked the topic a bit, but I'll continue it since we're already slipping down the slope. 😅

@ziebelje At this point, with the changes that Ecobee has made, what do you recommend as far as getting setup for local development? I think fake/stubbed data is very reasonable as a placeholder or a polyfill for some forms of dev, particularly for UI or operational stuff that doesn't depend on the API itself.

@ziebelje
Copy link
Member

Unfortunately I don't have a good solution right now. It might be possible to kind of get something going if you manually inject some data into it. If you'd like to try email me at [email protected] with your thermostat serial number and I can do a SQL data export for your user which you can import into your local database. Then you would have to bypass the authentication - also possible - and I think at that point you would be up and running.

@ziebelje ziebelje added the Self-Hosting Related to self-hosting beestat. label Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Self-Hosting Related to self-hosting beestat.
Projects
None yet
Development

No branches or pull requests

3 participants