lvanderree / com.synplyworks.thenmore

Homey Then functionality
GNU General Public License v3.0
0 stars 4 forks source link

Timer app crashes for v0.6.2 #10

Closed andrewbadge closed 5 years ago

andrewbadge commented 5 years ago

Hi, Firstly; love you work. I really appreciate the effort. To the Issue; I have several timers that turn on a group of Hue lights (using the < Group >) app. Periodically the Timer app crashes.

A friend has exactly the same setup but has not used the Group app. He just creates many timers i the each Flow. so i'm suspecting its the combination of your beta Timer and the Beta app.

I can send the Diagnostics report if you need them. Thanks. Andrew

lvanderree commented 5 years ago

A diagnostics report can certainly help, when it provides a crash stracktrace, together with your info here.

I don't have Hue lights, but I can try to reproduce it when I setup a flow with the Group app.

andrewbadge commented 5 years ago

Hi, Here is the report code: f4d50d9b-cf0d-480b-9397-888fe280293a (not sure if this is a security risk posing here...too late).

There is a detailed error message that I'll capture when it happens next. Sorry for not doing this previously.

Thanks

lvanderree commented 5 years ago

Thanks for the report, I've seen it in my email, so don't think the report code will introduce a security risk. It is simply the output of the log messages that you can see in the console when you run the app via athom-cli tools (reporting actions and device-ids)

However since there wasn't an error yet, this report only shows the correct working of the app, as far as I've seen quickly. Timer 0.6.2 did generate one crash report (with stacktrace) that got send to me at someone, but since I don't know what has happen, and at who's device it was hard for me to write a fix for it, since I cannot reproduce it yet

andrewbadge commented 5 years ago

Hi, Luckily (or not depending on your perspective) it crashed again. Photos including debug: https://photos.app.goo.gl/89xYjQ23YpmNbW5T8 Crash dump: 8c888f96-22b6-4748-b07d-ec4b97786f9b

Thanks

lvanderree commented 5 years ago

Yes thanks, now I've got the stacktrace in my mail as well, related to your setup. So I think I can try to reproduce it, or else let you try to reproduce it in a new version ;-)

I will keep you posted when I've made an update

GrimmJok3r commented 5 years ago

Hi Leon First thanks for this awesome app just saves me a lot of flows

But unfortunately it crashed I’m also running the beta 6.2

Here my crash info Thx Thijs

Hi!

Helaas is je app gecrashed toen ik het volgende deed:

(...)

Dit is de stack trace:(node:17110) UnhandledPromiseRejectionWarning: Philips Hue: 201, parameter, bri, is not modifiable. Device is set to off.: Philips Hue: 201, parameter, bri, is not modifiable. Device is set to off. at /node_modules/athom-api/dist/index.js:1:1044257 at at process._tickCallback (internal/process/next_tick.js:189:7) (node:17110) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1) (node:17110) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code. (node:17110) UnhandledPromiseRejectionWarning: Philips Hue: 201, parameter, bri, is not modifiable. Device is set to off.: Philips Hue: 201, parameter, bri, is not modifiable. Device is set to off. at /node_modules/athom-api/dist/index.js:1:1044257 at at process._tickCallback (internal/process/next_tick.js:189:7) (node:17110) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2) /app.js:204 this.timers[device.id].onOffCapabilityInstance.destroy(); ^

TypeError: Cannot read property 'onOffCapabilityInstance' of undefined at TimerApp. (/app.js:204:28) at ontimeout (timers.js:498:11) at tryOnTimeout (timers.js:323:5) at Timer.listOnTimeout (timers.js:290:5)

andrewbadge commented 5 years ago

Hi Leon,

FYI: I changed my flows to not use Groups for lights. I created individual timers per light to see if that resolved the issue. It didn't; I'm still getting crashes. So the cause is not neccessarily related to the < Group > app but something else. screenshot_20190203-204128

andrewbadge commented 5 years ago

Hi, Forgive me if this isn't helpful; but I'm thinking the root cause of the exception is the Homey API (and some random behavior). But at the same time no exceptions are caught in the timer code.

ie. looking at app.js there aren't any catch statements Hence any device exception is bubbling up to kill the app.

As a user; my preference is the exception is caught (logged) and the app doesn't crash. As the next flow would then work and resolve any device state issue. Currently the app crashes and i have to dig down to find and restart the app (and this usually happen at 2AM).

There may be a good reason that isn't done in practice... Regards, Andrew

lvanderree commented 5 years ago

Hi, I made some updates yesterday, but am testing them myself at the moment.

It seems like the Homey API is indeed introducing some problems:

I think this introduces some situations I did not anticipated on. Adding exception handlers can overcome these situations. I however did not do this (yet), since I explicitly wanted to know about these situations, since in that case the app will behave unpredictable.

I am currently removing a lot of overhead (I hope) and complexity from my application, by removing my cache instances. I have seen that the API-library also caches data, and until now I haven't noticed a decrease in performance yet, so I guess that was unnecessary. I also found out about yet another (Homey 2) API change, which caused a problem during filtering when selecting a device that is now fixed (zone name isn't directly available anymore).

Besides this, I noticed two different crash causes in the reports:

  1. There are issues with setting a capability, that cannot be set, even though I asked in advance if it can be set ( device.capabilitiesObj.dim.setable ) this looks to me like a driver issue of the device, which I can mitigate by catching the exception
  2. There are issues with getting the reference of the running timer, which isn't there anymore. This is probably causes by my app, setting a timer for a device twice, while it should have cancelled the first running timer, before setting the second. This might be a threading issue, which I can solve by simply first testing if the reference is still there. However I would like to be able to reproduce it, to be sure I did not oversee anything.

I pushed my changes to the beta branch, but didn't released it yet, so you have to sideload it if you want to test it

andrewbadge commented 5 years ago

Ok. That all makes sense. I'll side load and let you know how I go.

PS. great response. You really need a "buy me a beer" link ;-)

lvanderree commented 5 years ago

that was one of the other changes I made ;-) https://github.com/lvanderree/com.synplyworks.thenmore/commit/e8bce90ebba50c4492fce3c8958f90b2dbb46f8e#diff-04c6e90faac2675aa89e2176d2eec7d8R80

andrewbadge commented 5 years ago

FYI: I've side loaded the 0.7.0 version. i have got an exception already but I'm restarting Homey and will let you know if it happens after that. Thanks

lvanderree commented 5 years ago

Please provide the stack traces of these exceptions as well, since it will reference to different line numbers (and different code of course)

lvanderree commented 5 years ago

0.7.1 is pushed to git, which probably shouldn't crash anymore. I am however not sure if I always get the most current capability values from a device. Maybe I should read out values in a different way with: https://developer.athom.com/docs/api/HomeyAPI.ManagerDevices.html#getCapability but I don't know what uri is yet...

Ps. this should be sideloaded again, since I want to test it myself somewhat more, before releasing it at the store

lvanderree commented 5 years ago

that was one of the other changes I made ;-) e8bce90#diff-04c6e90faac2675aa89e2176d2eec7d8R80

That was a very nice beer

andrewbadge commented 5 years ago

Hi, Here is the crash from last night. I believe its currently still working after the restart. screenshot_20190207-104028

andrewbadge commented 5 years ago

that was one of the other changes I made ;-) e8bce90#diff-04c6e90faac2675aa89e2176d2eec7d8R80

That was a very nice beer

No problem. Thank you for your work.

lvanderree commented 5 years ago

Hi, Here is the crash from last night. I believe its currently still working after the restart.

Ik guess that should be fixed in 0.7.1 (the crash was from 0.7.0) I now log when I end up in these situations, which can help to investigate when unexpected behavior is detected, and hopefully can be reproduced.

lvanderree commented 5 years ago

I added your email adres to the alpha testers (next to me myself and I ;-)) at http://apps.athom.com so you should be able to install it from overthere

andrewbadge commented 5 years ago

Thanks. No crash so far with 0.7.1. I'm away for the weekend but I'll keep and eye on it.

andrewbadge commented 5 years ago

Hi Leon, Back from my trip away. No crashes since 0.7.1. Good news so far.

My parents were here so it should have triggered lots of flows but I'll let you know after a couple of days with me home.

lvanderree commented 5 years ago

I noticed that there is a bug in 0.7.1 that causes less flows triggering than expected/hoped, due to a cache running behind in the athom library that gets devices (including state, that never gets updated after the first retrieval). I made a work around for that in 0.7.2, and will implement a capability listener for this in the next version. So for new please update to 0.7.2

andrewbadge commented 5 years ago

Updated 0.7.2. Thanks

lvanderree commented 5 years ago

0.7.2 has just been published in the app store. I will be keeping this ticket, untill I implemented the capability listener, but I expect 0.7.2 to be working pretty stable.

I only expect that externally changed capability values (on/off or dim level) can cause unexpected behavior for now.

GrimmJok3r commented 5 years ago

Thx

Just updated it Will let You know either way

Thijs

Sent from the nether regions

On 11 Feb 2019, at 13:23, Leon van der Ree notifications@github.com<mailto:notifications@github.com> wrote:

0.7.2 has just been published in the app store. I will be keeping this ticket, untill I implemented the capability listener, but I expect 0.7.2 to be working pretty stable.

I only expect that externally changed capability values (on/off or dim level) can cause unexpected behavior for now.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/lvanderree/com.synplyworks.thenmore/issues/10#issuecomment-462309467, or mute the threadhttps://github.com/notifications/unsubscribe-auth/Adw3jhEejFANOfN6ZzhROGiMv1LPZ82Iks5vMWDYgaJpZM4aXJ2g.

andrewbadge commented 5 years ago

FYI: no exceptions since 0.7.2. Its been stable for 5 days. My issue seems closed.

Thanks Leon

lvanderree commented 5 years ago

Good to hear. Ik will probably change some things regarding the listeners in the future, but will close this ticket for now then!

GrimmJok3r commented 5 years ago

I haven’t had any issue’s sinds the 72 update.

Thx 😃

From: Leon van der Ree notifications@github.com Reply-To: "lvanderree/com.synplyworks.thenmore" reply@reply.github.com Date: Monday, 18 February 2019 at 08:49 To: "lvanderree/com.synplyworks.thenmore" com.synplyworks.thenmore@noreply.github.com Cc: Thijs Carnal thijs@levally.com, Comment comment@noreply.github.com Subject: Re: [lvanderree/com.synplyworks.thenmore] Timer app crashes for v0.6.2 (#10)

Good to hear. Ik will probably change some things regarding the listeners in the future, but will close this ticket for now then!

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/lvanderree/com.synplyworks.thenmore/issues/10#issuecomment-464621992, or mute the threadhttps://github.com/notifications/unsubscribe-auth/Adw3jkzu6DQnSrMAR05-a5DQ5zfeJ5Euks5vOlr1gaJpZM4aXJ2g.