File tree Expand file tree Collapse file tree 1 file changed +16
-1
lines changed Expand file tree Collapse file tree 1 file changed +16
-1
lines changed Original file line number Diff line number Diff line change @@ -137,10 +137,25 @@ ${metricsToLogString}
137
137
customMetrics . forEach ( metricName => {
138
138
let wptCustomMetric = firstViewData [ `_${ metricName } ` ] ;
139
139
try {
140
+ // Some, but not all, custom metrics wrap their return values in JSON.stringify()
141
+ // Some just return the objects. And some have strings which are not JSON!
142
+ // Gotta love the consistency!!!
143
+ //
144
+ // IMHO wrapping the return in JSON.stringify() is best practice, since WebPageTest
145
+ // will do that to save to database and you can run into weird edge cases where it
146
+ // doesn't return data when doing that, so explicitly doing that avoids that when
147
+ // testing in the console* , but we live in an inconsistent world!
148
+ // (* see https://github.com/HTTPArchive/custom-metrics/pull/113#issuecomment-2043937823)
149
+ //
150
+ // Anyway, if it's a string, see if we can parse it back to a JS object to pretty
151
+ // print it, but be prepared for that to fail for non-JSON strings so wrap in try/catch
152
+ // See pipeline: https://github.com/HTTPArchive/data-pipeline/blob/main/modules/import_all.py#L115
140
153
if ( typeof wptCustomMetric === 'string' ) {
141
154
wptCustomMetric = JSON . parse ( wptCustomMetric ) ;
142
155
}
143
- } catch ( e ) { }
156
+ } catch ( e ) {
157
+ // If it fails, that's OK as we'll just stick with the exact string output anyway
158
+ }
144
159
wptCustomMetrics [ `_${ metricName } ` ] = wptCustomMetric ;
145
160
} ) ;
146
161
You can’t perform that action at this time.
0 commit comments