Closed Bug 1442700 Opened 2 years ago Closed Last year
Add a label to TELEMETRY
_SEND _FAILURE _TYPE and Telemetry Send's XHR _ERROR _TYPE for e Terminated
Bug 1442700 : Label added to TELEMETRY_SEND_FAILURE_TYPE and TelemetrySend's XHR_ERROR_TYPE for eTerminated ,
59 bytes, text/x-review-board-request
bug 1400748 added a new ErrorType "eTerminated" for XMLHttpRequests in C++, but we didn't mirror it in TelemetrySend's XHR_ERROR_TYPE enum or TELEMETRY_SEND_FAILURE_TYPE's labels. This bug is to add it in. Specifically, find TelemetrySend's XHR_ERROR_TYPE and add in an entry for eTerminated and add a label to TELEMETRY_SEND_FAILURE_TYPE for eTerminated. For bonus points, add a comment in XMLHttpRequestMainThread.h on the ErrorType enum to ask that future additions to ErrorType be mirrored in TelemetrySend, and a comment in TelemetrySend that future additions to XHR_ERROR_TYPE be mirrored in TELEMETRY_SEND_FAILURE_TYPE. : https://searchfox.org/mozilla-central/rev/61d400da1c692453c2dc2c1cf37b616ce13dea5b/dom/xhr/XMLHttpRequestMainThread.h#194
HI Chris, I would like to take care of this enum issue. would you assign it to me ?
Certainly. Please let me know if you have questions. Note: This isn't as easy as some of the other good first bugs we have in our component, so if you are having trouble you might find one of them a better place to start.
Assignee: nobody → phsangeetha1
Status: NEW → ASSIGNED
Unassigning for now.
Assignee: phsangeetha1 → nobody
Status: ASSIGNED → NEW
Can i upload a patch for this?
Comment on attachment 8962142 [details] Bug 1442700 : Label added to TELEMETRY_SEND_FAILURE_TYPE and TelemetrySend's XHR_ERROR_TYPE for eTerminated , https://reviewboard.mozilla.org/r/230992/#review236600 This is exactly what we need, with just a few things that need cleaning up. ::: dom/xhr/XMLHttpRequestMainThread.h:188 (Diff revision 1) > load, > loadend, > ENUM_MAX > }; > > + // Make sure that any additions done to ErrorType enum are also mirrored in There's one space at the end of the line here that should be removed. ::: toolkit/components/telemetry/Histograms.json:7244 (Diff revision 1) > "labels": [ > "eOK", > "eRequest", > "eUnreachable", > "eChannelOpen", > + "eTerminated", This has to be added at the end of the list, as categorical histograms can only expand from the end.
Attachment #8962142 - Flags: review?(chutten)
Comment on attachment 8962636 [details] Bug 1442700 : Label added to TELEMETRY_SEND_FAILURE_TYPE and TelemetrySend's XHR_ERROR_TYPE for eTerminated V2 , https://reviewboard.mozilla.org/r/231496/#review237186 ::: toolkit/components/telemetry/Histograms.json:7248 (Diff revision 1) > "eChannelOpen", > "eRedirect", > "abort", > "timeout", > "eTooLate" > + "eTerminated" This won't build, as it is missing a comma on the preceding line. Be sure to build and test your changes before submitting a patch. (I forget sometimes, too)
Attachment #8962636 - Flags: review?(chutten) → review-
Comment on attachment 8963035 [details] Bug 1442700 : Label added to TELEMETRY_SEND_FAILURE_TYPE and TelemetrySend's XHR_ERROR_TYPE for eTerminated , https://reviewboard.mozilla.org/r/231870/#review237666 This looks good, thank you! How did you test this?
Attachment #8963035 - Flags: review?(chutten) → review+
Thanks Chris. For testing this i ran the `mach test toolkit/components/telemetry/tests/..` & `mach lint commands`.
Excellent, thank you for this. Let's mark this for checkin, shall we?
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/c1aa3f6a5a23 Label added to TELEMETRY_SEND_FAILURE_TYPE and TelemetrySend's XHR_ERROR_TYPE for eTerminated , r=chutten
You need to log in before you can comment on or make changes to this bug.