Update feature detection example #341
Conversation
|
@kenchris to also review the feature detection example code. |
| } else if (error.name === 'ReferenceError') { | ||
| console.log('Sensor is not supported by the User Agent.'); | ||
| } else { | ||
| throw error; |
There was a problem hiding this comment.
Why is throwing considered here?
There was a problem hiding this comment.
If there are other errors that handled in the if / else blocks, I think it is better to re-throw them, rather than supressing or logging.
There was a problem hiding this comment.
throwing doesn't make it robust though, an error happened and what ever happened you handled it... throwing can end up affecting other systems, like if someone else ends up catching it
There was a problem hiding this comment.
This is outside of scope of this example. Robust application should not suppress error propagation to avoid unpredictable behavior. I can fix throwing from error handler.
index.bs
Outdated
| accelerometer.addEventListener('error', event => handleErrors(event.error)); | ||
| accelerometer.addEventListener('reading', () => reloadOnShake(accelerometer)); | ||
| accelerometer.start(); | ||
| } catch (error) { |
There was a problem hiding this comment.
Maybe comment like // Handle construction errors
index.bs
Outdated
| let accelerometer = null; | ||
| try { | ||
| accelerometer = new Accelerometer({ frequency: 10 }); | ||
| accelerometer.addEventListener('error', event => handleErrors(event.error)); |
There was a problem hiding this comment.
Maybe comment // handle runtime errors
There was a problem hiding this comment.
what happens when handleErrors throws from a runtime error?
There was a problem hiding this comment.
log to console :) unhandled exception
There was a problem hiding this comment.
Would be nice to add that as a comment above the throw
There was a problem hiding this comment.
Then the throw makes even less sense... why not print to console straight away in both cases, you could use console.error()
There was a problem hiding this comment.
Added comment for runtime errors.
index.bs
Outdated
| handleErrors(error); | ||
| } | ||
|
|
||
| function handleErrors(error) { |
There was a problem hiding this comment.
Its called handleErrors but not doing much handling :-) more like logging
There was a problem hiding this comment.
Well, it is an example and not a full blown app :) Do you have some suggestions? It is app specific, to define behavior for particular error condition. E.g, fallback to DevMotion, show notification, etc.
index.bs
Outdated
| } | ||
|
|
||
| function handleErrors(error) { | ||
| if (error.name === 'NotAllowedError') { |
There was a problem hiding this comment.
// Do proper error handling here, like show notification, or fallback to other sensor;
// whatever makes the most sense for your application scenario.
There was a problem hiding this comment.
Added text to example description.
alexshalamov
force-pushed
the
improve_feature_detection
branch
from
bdae54b
to
45476e6
Compare
alexshalamov
force-pushed
the
improve_feature_detection
branch
from
45476e6
to
00b67d5
Compare
Fixes: #339
Preview | Diff