Improve KML network error handling

[thw0rted]

Sandcastle example: here

Browser: any

Operating System: any

The Sandcastle example makes a simple NetworkLink-based KML data source then "breaks" the network resource to simulate a real error. There are a couple of issues with how this plays out:

• The error event fires once with a RequestErrorEvent structure but it's empty.
• Nothing passed to the event callback tells us which URL failed to load.
• The error event fires a second time with a user-facing string, but we have no easy way to tell the difference.
• The generated string includes "[object Object]" because it was computed using string concatenation against a Resource, which does not (but probably should?) have a toString method.

Ideally, any failed network calls performed by the KmlDataSource should fire an error event that tells us what failed to load and (if known) why. Each event type should pass one type of argument to the callback, not a mix of structures (RequestErrorEvent) and strings.

Note that my example doesn't simulate e.g. loading a billboard image from a bad URL, but that would be a good further test case to ensure consistency.

Also, note that if the network link fails to load once, the only way to "get back on track" is to call load() again. Failure to set up the NetworkLink means that this element is ignored. It would be nice to have a method we could call that would retry failed links without a full reload.

[OmarShehata]

I agree better error logging is a valuable investment!

[thw0rted]

Would the team support adding a simple Resource#toString that just returns the URL? I'm crazy busy lately but if I have time I could try to get together a PR and that would address a small part of this.

[mramato]

@thw0rted I honestly thought we already had one, but apparently it's the Resource.url property instead. If you want to open a PR that just overrides toString and returns the url property, I don't see why we wouldn't take it.

[thw0rted]

It's been a while! I was going through some old tickets to see if I could close them out, so I started trying to recreate some error conditions to see if I have all the information I need to give good user feedback on failure.

I found another problem right away: when Resource requests something that fails CORS protections, the calling code throws/rejects with a totally empty RequestErrorEvent. This is, of course, not very productive. In the original problem report, the caller was logging this empty RequestErrorEvent along with a string, but if KMLDataSource#load rejects due to a CORS failure it simply passes out the empty object with no other context (!).

It occurred to me that if RequestErrorEvent were required to include a url parameter, it would be a lot more useful. Having a required constructor param would be a breaking change, and technically the class is exported, but I suspect it wasn't really intended for "outside" use. Does that sound acceptable?