search

raquo / laminar-full-stack-demo

Fully working client + server, dev + prod setup for Scala.js development, showcasing a bunch of Laminar & Scala.js patterns.
https://demo.laminar.dev
MIT License
50 stars 11 forks source link

Upstream weather service partial outage is not handled #4

Open mkrauss opened 6 months ago

mkrauss commented 6 months ago

Thanks for the fantastic repo, this has been a very useful learning tool.

I had it working at first, but at some point the upstream API call at WeatherFetcher.scala line 26 is returning an empty currentConditions element. This would have been more confusing if it happened when I was first trying to get it going. When this problem first started, both Squamish and Netinat were failing. At the current moment, Netinat is working, but Squamish is still out.

I found this comment on a similar issue for a different region here (https://github.com/home-assistant/core/issues/101533#issuecomment-1758381641) so maybe this is something that the code should be able to more gracefully handle. Although I could see if you want to leave it as is to keep the example pure. Maybe even just make a few more stations available, and add a warning that they could be out would help?

raquo commented 5 months ago

Ah it seems that I am too late to debug this, they must have fixed the station outage already.

Do you remember what exactly you've seen in the API response? Was the API returning <currentConditions></currentConditions>? As in, the element was present, but had no contents?

If that's the case, it looks like it would be quite painful to properly represent that in the phobos XML decoders that we use.

I guess we could work around the issue by regex-finding <currentConditions>[whitespace]</currentConditions> in responseText in WeatherFetcher.scala:31...

mkrauss commented 5 months ago

Yes, the element was present but had no contents, definitely. If I recall correctly, I think it was actually <currentConditions/>, a single self-closing tag, but I'm not completely sure of that.

raquo commented 5 months ago

Good to know, thanks. Makes sense.

Ideally I'd rather fix this when I can see the error happening, so if anyone sees this error again on the weather gradient page, please ping me here. Otherwise, I'll get to it... eventually...