Amp-font: Un-handled Promise Rejection #1095 #1107
Conversation
| @@ -124,7 +128,7 @@ export class AmpFont extends AMP.BaseElement { | |||
| this.onFontLoadSuccess_(); | |||
| }).catch(error => { | |||
| this.onFontLoadError_(); | |||
| throw error; | |||
| log.warn(TAG_, error, this.element); | |||
There was a problem hiding this comment.
The logger isn't the right tool in its current form as content devs never see it.
@erwinmombay was looking into fixing that, but I believe for now we should use console.warn/log.
There was a problem hiding this comment.
@cramforce lets go with that, that'll make it easier for me to separate them.
There was a problem hiding this comment.
@cramforce using console.log/warn, i have a branch that separates amp-dev logs vs publisher-dev logs, so this makes it easier for me to follow up. (where console.log/warn will always be publisher-dev logs)
There was a problem hiding this comment.
@dvoytenko suggested log.warn - as opposed to console.warn.
I have added a warn. @cramforce Comment on which one needs removal now
Ideally it would be nice to have log.warn decide which ones should be console.warn'd, rather than using 2 different logging systems
There was a problem hiding this comment.
@sriramkrish85 @erwinmombay is working on unifying the 2.
For now add console.warn
camelburrito
force-pushed
the
fontbug
branch
3 times, most recently
from
53485ba
to
370d5a7
Compare
|
@sriramkrish85 LGTM |
Amp-font: Un-handled Promise Rejection #1095
Amp-font: Unhandled Promise Rejection #1095