Bug 1321790 - Change the JS Scalar API so that it doesn't throw. r=chutten
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Wed, 18 Jan 2017 06:26:00 -0500
changeset 377320 904be97288889c9c387bde5dcb9816cd3952bc6f
parent 377319 5fb91cd2e2fcf0ef047763809d8345feef99b80d
child 377321 95a36f9308abcb2d3cc700bc06397b0845f31da9
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1321790
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1321790 - Change the JS Scalar API so that it doesn't throw. r=chutten MozReview-Commit-ID: K8jVoLr8GJn
toolkit/components/telemetry/TelemetryScalar.cpp
toolkit/components/telemetry/tests/unit/test_TelemetryScalars.js
--- a/toolkit/components/telemetry/TelemetryScalar.cpp
+++ b/toolkit/components/telemetry/TelemetryScalar.cpp
@@ -78,17 +78,21 @@ const uint32_t kMaximumKeyStringLength =
 const uint32_t kMaximumStringValueLength = 50;
 const uint32_t kScalarCount =
   static_cast<uint32_t>(mozilla::Telemetry::ScalarID::ScalarCount);
 
 enum class ScalarResult : uint8_t {
   // Nothing went wrong.
   Ok,
   // General Scalar Errors
+  NotInitialized,
+  CannotUnpackVariant,
   CannotRecordInProcess,
+  KeyedTypeMismatch,
+  UnknownScalar,
   OperationNotSupported,
   InvalidType,
   InvalidValue,
   // Keyed Scalar Errors
   KeyTooLong,
   TooManyKeys,
   // String Scalar Errors
   StringTooLong,
@@ -97,48 +101,16 @@ enum class ScalarResult : uint8_t {
   UnsignedTruncatedValue
 };
 
 typedef nsBaseHashtableET<nsDepCharHashKey, mozilla::Telemetry::ScalarID>
           CharPtrEntryType;
 
 typedef AutoHashtable<CharPtrEntryType> ScalarMapType;
 
-/**
- * Map the error codes used internally to NS_* error codes.
- * @param aSr The error code used internally in this module.
- * @return {nsresult} A NS_* error code.
- */
-nsresult
-MapToNsResult(ScalarResult aSr)
-{
-  switch (aSr) {
-    case ScalarResult::Ok:
-    case ScalarResult::CannotRecordInProcess:
-      return NS_OK;
-    case ScalarResult::OperationNotSupported:
-      return NS_ERROR_NOT_AVAILABLE;
-    case ScalarResult::StringTooLong:
-      // We don't want to throw if we're setting a string that is too long.
-      return NS_OK;
-    case ScalarResult::InvalidType:
-    case ScalarResult::InvalidValue:
-    case ScalarResult::KeyTooLong:
-      return NS_ERROR_ILLEGAL_VALUE;
-    case ScalarResult::TooManyKeys:
-      return NS_ERROR_FAILURE;
-    case ScalarResult::UnsignedNegativeValue:
-    case ScalarResult::UnsignedTruncatedValue:
-      // We shouldn't throw if trying to set a negative number or are truncated,
-      // only warn the user.
-      return NS_OK;
-  }
-  return NS_ERROR_FAILURE;
-}
-
 bool
 IsValidEnumId(mozilla::Telemetry::ScalarID aID)
 {
   return aID < mozilla::Telemetry::ScalarID::ScalarCount;
 }
 
 /**
  * The following helpers are used to get a nsIVariant from a uint32_t,
@@ -778,59 +750,53 @@ ProcessesKeyedScalarsMapType gKeyedScala
 // back into the TelemetryScalar interface, hence trying to re-acquire the mutex.
 //
 // This means that these functions potentially race against threads, but
 // that seems preferable to risking deadlock.
 
 namespace {
 
 /**
- * Checks if the error should be logged.
- *
- * @param aSr The error code.
- * @return true if the error should be logged, false otherwise.
- */
-bool
-internal_ShouldLogError(ScalarResult aSr)
-{
-  switch (aSr) {
-    case ScalarResult::CannotRecordInProcess: MOZ_FALLTHROUGH;
-    case ScalarResult::StringTooLong: MOZ_FALLTHROUGH;
-    case ScalarResult::KeyTooLong: MOZ_FALLTHROUGH;
-    case ScalarResult::TooManyKeys: MOZ_FALLTHROUGH;
-    case ScalarResult::UnsignedNegativeValue: MOZ_FALLTHROUGH;
-    case ScalarResult::UnsignedTruncatedValue:
-      // Intentional fall-through.
-      return true;
-
-    default:
-      return false;
-  }
-
-  // It should never reach this point.
-  return false;
-}
-
-/**
  * Converts the error code to a human readable error message and prints it to the
  * browser console.
  *
  * @param aScalarName The name of the scalar that raised the error.
  * @param aSr The error code.
  */
 void
 internal_LogScalarError(const nsACString& aScalarName, ScalarResult aSr)
 {
   nsAutoString errorMessage;
   AppendUTF8toUTF16(aScalarName, errorMessage);
 
   switch (aSr) {
+    case ScalarResult::NotInitialized:
+      errorMessage.Append(NS_LITERAL_STRING(" - Telemetry was not yet initialized."));
+      break;
+    case ScalarResult::CannotUnpackVariant:
+      errorMessage.Append(NS_LITERAL_STRING(" - Cannot convert the provided JS value to nsIVariant."));
+      break;
     case ScalarResult::CannotRecordInProcess:
       errorMessage.Append(NS_LITERAL_STRING(" - Cannot record the scalar in the current process."));
       break;
+    case ScalarResult::KeyedTypeMismatch:
+      errorMessage.Append(NS_LITERAL_STRING(" - Attempting to manage a keyed scalar as a scalar (or vice-versa)."));
+      break;
+    case ScalarResult::UnknownScalar:
+      errorMessage.Append(NS_LITERAL_STRING(" - Unknown scalar."));
+      break;
+    case ScalarResult::OperationNotSupported:
+      errorMessage.Append(NS_LITERAL_STRING(" - The requested operation is not supported on this scalar."));
+      break;
+    case ScalarResult::InvalidType:
+      errorMessage.Append(NS_LITERAL_STRING(" - Attempted to set the scalar to an invalid data type."));
+      break;
+    case ScalarResult::InvalidValue:
+      errorMessage.Append(NS_LITERAL_STRING(" - Attempted to set the scalar to an incompatible value."));
+      break;
     case ScalarResult::StringTooLong:
       errorMessage.Append(NS_LITERAL_STRING(" - Truncating scalar value to 50 characters."));
       break;
     case ScalarResult::KeyTooLong:
       errorMessage.Append(NS_LITERAL_STRING(" - The key length must be limited to 70 characters."));
       break;
     case ScalarResult::TooManyKeys:
       errorMessage.Append(NS_LITERAL_STRING(" - Keyed scalars cannot have more than 100 keys."));
@@ -1044,16 +1010,77 @@ internal_GetRecordableScalar(mozilla::Te
   // Are we allowed to record this scalar?
   if (!internal_CanRecordForScalarID(aId) || !internal_CanRecordProcess(aId)) {
     return nullptr;
   }
 
   return scalar;
 }
 
+/**
+ * Update the scalar with the provided value. This is used by the JS API.
+ *
+ * @param aName The scalar name.
+ * @param aType The action type for updating the scalar.
+ * @param aValue The value to use for updating the scalar.
+ * @return a ScalarResult error value.
+ */
+ScalarResult
+internal_UpdateScalar(const nsACString& aName, ScalarActionType aType,
+                      nsIVariant* aValue)
+{
+  mozilla::Telemetry::ScalarID id;
+  nsresult rv = internal_GetEnumByScalarName(aName, &id);
+  if (NS_FAILED(rv)) {
+    return (rv == NS_ERROR_FAILURE) ?
+           ScalarResult::NotInitialized : ScalarResult::UnknownScalar;
+  }
+
+  // We're trying to set a plain scalar, so make sure this is one.
+  if (internal_IsKeyedScalar(id)) {
+    return ScalarResult::KeyedTypeMismatch;
+  }
+
+  // Are we allowed to record this scalar?
+  if (!internal_CanRecordForScalarID(id)) {
+    return ScalarResult::Ok;
+  }
+
+  // Can we record in this process?
+  if (!internal_CanRecordProcess(id)) {
+    return ScalarResult::CannotRecordInProcess;
+  }
+
+  // Accumulate in the child process if needed.
+  if (!XRE_IsParentProcess()) {
+    const ScalarInfo &info = gScalars[static_cast<uint32_t>(id)];
+    TelemetryIPCAccumulator::RecordChildScalarAction(id, info.kind, aType, aValue);
+    return ScalarResult::Ok;
+  }
+
+  // Finally get the scalar.
+  ScalarBase* scalar = nullptr;
+  rv = internal_GetScalarByEnum(id, GeckoProcessType_Default, &scalar);
+  if (NS_FAILED(rv)) {
+    // Don't throw on expired scalars.
+    if (rv == NS_ERROR_NOT_AVAILABLE) {
+      return ScalarResult::Ok;
+    }
+    return ScalarResult::UnknownScalar;
+  }
+
+  if (aType == ScalarActionType::eAdd) {
+    return scalar->AddValue(aValue);
+  } else if (aType == ScalarActionType::eSet) {
+    return scalar->SetValue(aValue);
+  }
+
+  return scalar->SetMaximum(aValue);
+}
+
 } // namespace
 
 
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
 //
 // PRIVATE: thread-unsafe helpers for the keyed scalars
@@ -1152,16 +1179,78 @@ internal_GetRecordableKeyedScalar(mozill
   // Are we allowed to record this scalar?
   if (!internal_CanRecordForScalarID(aId) || !internal_CanRecordProcess(aId)) {
     return nullptr;
   }
 
   return scalar;
 }
 
+/**
+ * Update the keyed scalar with the provided value. This is used by the JS API.
+ *
+ * @param aName The scalar name.
+ * @param aKey The key name.
+ * @param aType The action type for updating the scalar.
+ * @param aValue The value to use for updating the scalar.
+ * @return a ScalarResult error value.
+ */
+ScalarResult
+internal_UpdateKeyedScalar(const nsACString& aName, const nsAString& aKey,
+                           ScalarActionType aType, nsIVariant* aValue)
+{
+  mozilla::Telemetry::ScalarID id;
+  nsresult rv = internal_GetEnumByScalarName(aName, &id);
+  if (NS_FAILED(rv)) {
+    return (rv == NS_ERROR_FAILURE) ?
+           ScalarResult::NotInitialized : ScalarResult::UnknownScalar;
+  }
+
+  // We're trying to set a keyed scalar, so make sure this is one.
+  if (!internal_IsKeyedScalar(id)) {
+    return ScalarResult::KeyedTypeMismatch;
+  }
+
+  // Are we allowed to record this scalar?
+  if (!internal_CanRecordForScalarID(id)) {
+    return ScalarResult::Ok;
+  }
+
+  // Can we record in this process?
+  if (!internal_CanRecordProcess(id)) {
+    return ScalarResult::CannotRecordInProcess;
+  }
+
+  // Accumulate in the child process if needed.
+  if (!XRE_IsParentProcess()) {
+    const ScalarInfo &info = gScalars[static_cast<uint32_t>(id)];
+    TelemetryIPCAccumulator::RecordChildKeyedScalarAction(id, aKey, info.kind, aType, aValue);
+    return ScalarResult::Ok;
+  }
+
+  // Finally get the scalar.
+  KeyedScalar* scalar = nullptr;
+  rv = internal_GetKeyedScalarByEnum(id, GeckoProcessType_Default, &scalar);
+  if (NS_FAILED(rv)) {
+    // Don't throw on expired scalars.
+    if (rv == NS_ERROR_NOT_AVAILABLE) {
+      return ScalarResult::Ok;
+    }
+    return ScalarResult::UnknownScalar;
+  }
+
+  if (aType == ScalarActionType::eAdd) {
+    return scalar->AddValue(aKey, aValue);
+  } else if (aType == ScalarActionType::eSet) {
+    return scalar->SetValue(aKey, aValue);
+  }
+
+  return scalar->SetMaximum(aKey, aValue);
+}
+
 } // namespace
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
 //
 // EXTERNALLY VISIBLE FUNCTIONS in namespace TelemetryScalars::
 
 // This is a StaticMutex rather than a plain Mutex (1) so that
@@ -1226,158 +1315,80 @@ TelemetryScalar::SetCanRecordExtended(bo
 }
 
 /**
  * Adds the value to the given scalar.
  *
  * @param aName The scalar name.
  * @param aVal The numeric value to add to the scalar.
  * @param aCx The JS context.
- * @return NS_OK if the value was added or if we're not allowed to record to this
- *  dataset. Otherwise, return an error.
+ * @return NS_OK (always) so that the JS API call doesn't throw. In case of errors,
+ *         a warning level message is printed in the browser console.
  */
 nsresult
 TelemetryScalar::Add(const nsACString& aName, JS::HandleValue aVal, JSContext* aCx)
 {
   // Unpack the aVal to nsIVariant. This uses the JS context.
   nsCOMPtr<nsIVariant> unpackedVal;
   nsresult rv =
     nsContentUtils::XPConnect()->JSToVariant(aCx, aVal,  getter_AddRefs(unpackedVal));
   if (NS_FAILED(rv)) {
-    return rv;
+    internal_LogScalarError(aName, ScalarResult::CannotUnpackVariant);
+    return NS_OK;
   }
 
   ScalarResult sr;
   {
     StaticMutexAutoLock locker(gTelemetryScalarsMutex);
-
-    mozilla::Telemetry::ScalarID id;
-    rv = internal_GetEnumByScalarName(aName, &id);
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-
-    // We're trying to set a plain scalar, so make sure this is one.
-    if (internal_IsKeyedScalar(id)) {
-      return NS_ERROR_ILLEGAL_VALUE;
-    }
-
-    // Are we allowed to record this scalar?
-    if (!internal_CanRecordForScalarID(id)) {
-      return NS_OK;
-    }
-
-    if (internal_CanRecordProcess(id)) {
-      // Accumulate in the child process if needed.
-      if (!XRE_IsParentProcess()) {
-        const ScalarInfo &info = gScalars[static_cast<uint32_t>(id)];
-        TelemetryIPCAccumulator::RecordChildScalarAction(id, info.kind, ScalarActionType::eAdd,
-                                                         unpackedVal);
-        return NS_OK;
-      }
-
-      // Finally get the scalar.
-      ScalarBase* scalar = nullptr;
-      rv = internal_GetScalarByEnum(id, GeckoProcessType_Default, &scalar);
-      if (NS_FAILED(rv)) {
-        // Don't throw on expired scalars.
-        if (rv == NS_ERROR_NOT_AVAILABLE) {
-          return NS_OK;
-        }
-        return rv;
-      }
-
-      sr = scalar->AddValue(unpackedVal);
-    } else {
-      sr = ScalarResult::CannotRecordInProcess;
-    }
+    sr = internal_UpdateScalar(aName, ScalarActionType::eAdd, unpackedVal);
   }
 
   // Warn the user about the error if we need to.
-  if (internal_ShouldLogError(sr)) {
+  if (sr != ScalarResult::Ok) {
     internal_LogScalarError(aName, sr);
   }
 
-  return MapToNsResult(sr);
+  return NS_OK;
 }
 
 /**
  * Adds the value to the given scalar.
  *
  * @param aName The scalar name.
  * @param aKey The key name.
  * @param aVal The numeric value to add to the scalar.
  * @param aCx The JS context.
- * @return NS_OK if the value was added or if we're not allow to record to this
- *  dataset. Otherwise, return an error.
+ * @return NS_OK (always) so that the JS API call doesn't throw. In case of errors,
+ *         a warning level message is printed in the browser console.
  */
 nsresult
 TelemetryScalar::Add(const nsACString& aName, const nsAString& aKey, JS::HandleValue aVal,
                      JSContext* aCx)
 {
   // Unpack the aVal to nsIVariant. This uses the JS context.
   nsCOMPtr<nsIVariant> unpackedVal;
   nsresult rv =
     nsContentUtils::XPConnect()->JSToVariant(aCx, aVal,  getter_AddRefs(unpackedVal));
   if (NS_FAILED(rv)) {
-    return rv;
+    internal_LogScalarError(aName, ScalarResult::CannotUnpackVariant);
+    return NS_OK;
   }
 
   ScalarResult sr;
   {
     StaticMutexAutoLock locker(gTelemetryScalarsMutex);
-
-    mozilla::Telemetry::ScalarID id;
-    rv = internal_GetEnumByScalarName(aName, &id);
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-
-    // Make sure this is a keyed scalar.
-    if (!internal_IsKeyedScalar(id)) {
-      return NS_ERROR_ILLEGAL_VALUE;
-    }
-
-    // Are we allowed to record this scalar?
-    if (!internal_CanRecordForScalarID(id)) {
-      return NS_OK;
-    }
-
-    if (internal_CanRecordProcess(id)) {
-      // Accumulate in the child process if needed.
-      if (!XRE_IsParentProcess()) {
-        const ScalarInfo &info = gScalars[static_cast<uint32_t>(id)];
-        TelemetryIPCAccumulator::RecordChildKeyedScalarAction(
-          id, aKey, info.kind, ScalarActionType::eAdd, unpackedVal);
-        return NS_OK;
-      }
-
-      // Finally get the scalar.
-      KeyedScalar* scalar = nullptr;
-      rv = internal_GetKeyedScalarByEnum(id, GeckoProcessType_Default, &scalar);
-      if (NS_FAILED(rv)) {
-        // Don't throw on expired scalars.
-        if (rv == NS_ERROR_NOT_AVAILABLE) {
-          return NS_OK;
-        }
-        return rv;
-      }
-
-      sr = scalar->AddValue(aKey, unpackedVal);
-    } else {
-      sr = ScalarResult::CannotRecordInProcess;
-    }
+    sr = internal_UpdateKeyedScalar(aName, aKey, ScalarActionType::eAdd, unpackedVal);
   }
 
   // Warn the user about the error if we need to.
-  if (internal_ShouldLogError(sr)) {
+  if (sr != ScalarResult::Ok) {
     internal_LogScalarError(aName, sr);
   }
 
-  return MapToNsResult(sr);
+  return NS_OK;
 }
 
 /**
  * Adds the value to the given scalar.
  *
  * @param aId The scalar enum id.
  * @param aVal The numeric value to add to the scalar.
  */
@@ -1442,158 +1453,80 @@ TelemetryScalar::Add(mozilla::Telemetry:
 }
 
 /**
  * Sets the scalar to the given value.
  *
  * @param aName The scalar name.
  * @param aVal The value to set the scalar to.
  * @param aCx The JS context.
- * @return NS_OK if the value was added or if we're not allow to record to this
- *  dataset. Otherwise, return an error.
+ * @return NS_OK (always) so that the JS API call doesn't throw. In case of errors,
+ *         a warning level message is printed in the browser console.
  */
 nsresult
 TelemetryScalar::Set(const nsACString& aName, JS::HandleValue aVal, JSContext* aCx)
 {
   // Unpack the aVal to nsIVariant. This uses the JS context.
   nsCOMPtr<nsIVariant> unpackedVal;
   nsresult rv =
     nsContentUtils::XPConnect()->JSToVariant(aCx, aVal,  getter_AddRefs(unpackedVal));
   if (NS_FAILED(rv)) {
-    return rv;
+    internal_LogScalarError(aName, ScalarResult::CannotUnpackVariant);
+    return NS_OK;
   }
 
   ScalarResult sr;
   {
     StaticMutexAutoLock locker(gTelemetryScalarsMutex);
-
-    mozilla::Telemetry::ScalarID id;
-    rv = internal_GetEnumByScalarName(aName, &id);
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-
-    // We're trying to set a plain scalar, so make sure this is one.
-    if (internal_IsKeyedScalar(id)) {
-      return NS_ERROR_ILLEGAL_VALUE;
-    }
-
-    // Are we allowed to record this scalar?
-    if (!internal_CanRecordForScalarID(id)) {
-      return NS_OK;
-    }
-
-    if (internal_CanRecordProcess(id)) {
-      // Accumulate in the child process if needed.
-      if (!XRE_IsParentProcess()) {
-        const ScalarInfo &info = gScalars[static_cast<uint32_t>(id)];
-        TelemetryIPCAccumulator::RecordChildScalarAction(id, info.kind, ScalarActionType::eSet,
-                                                         unpackedVal);
-        return NS_OK;
-      }
-
-      // Finally get the scalar.
-      ScalarBase* scalar = nullptr;
-      rv = internal_GetScalarByEnum(id, GeckoProcessType_Default, &scalar);
-      if (NS_FAILED(rv)) {
-        // Don't throw on expired scalars.
-        if (rv == NS_ERROR_NOT_AVAILABLE) {
-          return NS_OK;
-        }
-        return rv;
-      }
-
-      sr = scalar->SetValue(unpackedVal);
-    } else {
-      sr = ScalarResult::CannotRecordInProcess;
-    }
+    sr = internal_UpdateScalar(aName, ScalarActionType::eSet, unpackedVal);
   }
 
   // Warn the user about the error if we need to.
-  if (internal_ShouldLogError(sr)) {
+  if (sr != ScalarResult::Ok) {
     internal_LogScalarError(aName, sr);
   }
 
-  return MapToNsResult(sr);
+  return NS_OK;
 }
 
 /**
  * Sets the keyed scalar to the given value.
  *
  * @param aName The scalar name.
  * @param aKey The key name.
  * @param aVal The value to set the scalar to.
  * @param aCx The JS context.
- * @return NS_OK if the value was added or if we're not allow to record to this
- *  dataset. Otherwise, return an error.
+ * @return NS_OK (always) so that the JS API call doesn't throw. In case of errors,
+ *         a warning level message is printed in the browser console.
  */
 nsresult
 TelemetryScalar::Set(const nsACString& aName, const nsAString& aKey, JS::HandleValue aVal,
                      JSContext* aCx)
 {
   // Unpack the aVal to nsIVariant. This uses the JS context.
   nsCOMPtr<nsIVariant> unpackedVal;
   nsresult rv =
     nsContentUtils::XPConnect()->JSToVariant(aCx, aVal,  getter_AddRefs(unpackedVal));
   if (NS_FAILED(rv)) {
-    return rv;
+    internal_LogScalarError(aName, ScalarResult::CannotUnpackVariant);
+    return NS_OK;
   }
 
   ScalarResult sr;
   {
     StaticMutexAutoLock locker(gTelemetryScalarsMutex);
-
-    mozilla::Telemetry::ScalarID id;
-    rv = internal_GetEnumByScalarName(aName, &id);
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-
-    // We're trying to set a keyed scalar. Report an error if this isn't one.
-    if (!internal_IsKeyedScalar(id)) {
-      return NS_ERROR_ILLEGAL_VALUE;
-    }
-
-    // Are we allowed to record this scalar?
-    if (!internal_CanRecordForScalarID(id)) {
-      return NS_OK;
-    }
-
-    if (internal_CanRecordProcess(id)) {
-      // Accumulate in the child process if needed.
-      if (!XRE_IsParentProcess()) {
-        const ScalarInfo &info = gScalars[static_cast<uint32_t>(id)];
-        TelemetryIPCAccumulator::RecordChildKeyedScalarAction(
-          id, aKey, info.kind, ScalarActionType::eSet, unpackedVal);
-        return NS_OK;
-      }
-
-      // Finally get the scalar.
-      KeyedScalar* scalar = nullptr;
-      rv = internal_GetKeyedScalarByEnum(id, GeckoProcessType_Default, &scalar);
-      if (NS_FAILED(rv)) {
-        // Don't throw on expired scalars.
-        if (rv == NS_ERROR_NOT_AVAILABLE) {
-          return NS_OK;
-        }
-        return rv;
-      }
-
-      sr = scalar->SetValue(aKey, unpackedVal);
-    } else {
-      sr = ScalarResult::CannotRecordInProcess;
-    }
+    sr = internal_UpdateKeyedScalar(aName, aKey, ScalarActionType::eSet, unpackedVal);
   }
 
   // Warn the user about the error if we need to.
-  if (internal_ShouldLogError(sr)) {
+  if (sr != ScalarResult::Ok) {
     internal_LogScalarError(aName, sr);
   }
 
-  return MapToNsResult(sr);
+  return NS_OK;
 }
 
 /**
  * Sets the scalar to the given numeric value.
  *
  * @param aId The scalar enum id.
  * @param aValue The numeric, unsigned value to set the scalar to.
  */
@@ -1756,158 +1689,80 @@ TelemetryScalar::Set(mozilla::Telemetry:
 }
 
 /**
  * Sets the scalar to the maximum of the current and the passed value.
  *
  * @param aName The scalar name.
  * @param aVal The numeric value to set the scalar to.
  * @param aCx The JS context.
- * @return NS_OK if the value was added or if we're not allow to record to this
- *  dataset. Otherwise, return an error.
+ * @return NS_OK (always) so that the JS API call doesn't throw. In case of errors,
+ *         a warning level message is printed in the browser console.
  */
 nsresult
 TelemetryScalar::SetMaximum(const nsACString& aName, JS::HandleValue aVal, JSContext* aCx)
 {
   // Unpack the aVal to nsIVariant. This uses the JS context.
   nsCOMPtr<nsIVariant> unpackedVal;
   nsresult rv =
     nsContentUtils::XPConnect()->JSToVariant(aCx, aVal,  getter_AddRefs(unpackedVal));
   if (NS_FAILED(rv)) {
-    return rv;
+    internal_LogScalarError(aName, ScalarResult::CannotUnpackVariant);
+    return NS_OK;
   }
 
   ScalarResult sr;
   {
     StaticMutexAutoLock locker(gTelemetryScalarsMutex);
-
-    mozilla::Telemetry::ScalarID id;
-    rv = internal_GetEnumByScalarName(aName, &id);
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-
-    // Make sure this is not a keyed scalar.
-    if (internal_IsKeyedScalar(id)) {
-      return NS_ERROR_ILLEGAL_VALUE;
-    }
-
-    // Are we allowed to record this scalar?
-    if (!internal_CanRecordForScalarID(id)) {
-      return NS_OK;
-    }
-
-    if (internal_CanRecordProcess(id)) {
-      // Accumulate in the child process if needed.
-      if (!XRE_IsParentProcess()) {
-        const ScalarInfo &info = gScalars[static_cast<uint32_t>(id)];
-        TelemetryIPCAccumulator::RecordChildScalarAction(id, info.kind, ScalarActionType::eSetMaximum,
-                                                         unpackedVal);
-        return NS_OK;
-      }
-
-      // Finally get the scalar.
-      ScalarBase* scalar = nullptr;
-      rv = internal_GetScalarByEnum(id, GeckoProcessType_Default, &scalar);
-      if (NS_FAILED(rv)) {
-        // Don't throw on expired scalars.
-        if (rv == NS_ERROR_NOT_AVAILABLE) {
-          return NS_OK;
-        }
-        return rv;
-      }
-
-      sr = scalar->SetMaximum(unpackedVal);
-    } else {
-      sr = ScalarResult::CannotRecordInProcess;
-    }
+    sr = internal_UpdateScalar(aName, ScalarActionType::eSetMaximum, unpackedVal);
   }
 
   // Warn the user about the error if we need to.
-  if (internal_ShouldLogError(sr)) {
+  if (sr != ScalarResult::Ok) {
     internal_LogScalarError(aName, sr);
   }
 
-  return MapToNsResult(sr);
+  return NS_OK;
 }
 
 /**
  * Sets the scalar to the maximum of the current and the passed value.
  *
  * @param aName The scalar name.
  * @param aKey The key name.
  * @param aVal The numeric value to set the scalar to.
  * @param aCx The JS context.
- * @return NS_OK if the value was added or if we're not allow to record to this
- *  dataset. Otherwise, return an error.
+ * @return NS_OK (always) so that the JS API call doesn't throw. In case of errors,
+ *         a warning level message is printed in the browser console.
  */
 nsresult
 TelemetryScalar::SetMaximum(const nsACString& aName, const nsAString& aKey, JS::HandleValue aVal,
                             JSContext* aCx)
 {
   // Unpack the aVal to nsIVariant. This uses the JS context.
   nsCOMPtr<nsIVariant> unpackedVal;
   nsresult rv =
     nsContentUtils::XPConnect()->JSToVariant(aCx, aVal,  getter_AddRefs(unpackedVal));
   if (NS_FAILED(rv)) {
-    return rv;
+    internal_LogScalarError(aName, ScalarResult::CannotUnpackVariant);
+    return NS_OK;
   }
 
   ScalarResult sr;
   {
     StaticMutexAutoLock locker(gTelemetryScalarsMutex);
-
-    mozilla::Telemetry::ScalarID id;
-    rv = internal_GetEnumByScalarName(aName, &id);
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-
-    // Make sure this is a keyed scalar.
-    if (!internal_IsKeyedScalar(id)) {
-      return NS_ERROR_ILLEGAL_VALUE;
-    }
-
-    // Are we allowed to record this scalar?
-    if (!internal_CanRecordForScalarID(id)) {
-      return NS_OK;
-    }
-
-    if (internal_CanRecordProcess(id)) {
-      // Accumulate in the child process if needed.
-      if (!XRE_IsParentProcess()) {
-        const ScalarInfo &info = gScalars[static_cast<uint32_t>(id)];
-        TelemetryIPCAccumulator::RecordChildKeyedScalarAction(
-          id, aKey, info.kind, ScalarActionType::eSetMaximum, unpackedVal);
-        return NS_OK;
-      }
-
-      // Finally get the scalar.
-      KeyedScalar* scalar = nullptr;
-      rv = internal_GetKeyedScalarByEnum(id, GeckoProcessType_Default, &scalar);
-      if (NS_FAILED(rv)) {
-        // Don't throw on expired scalars.
-        if (rv == NS_ERROR_NOT_AVAILABLE) {
-          return NS_OK;
-        }
-        return rv;
-      }
-
-      sr = scalar->SetMaximum(aKey, unpackedVal);
-    } else {
-      sr = ScalarResult::CannotRecordInProcess;
-    }
+    sr = internal_UpdateKeyedScalar(aName, aKey, ScalarActionType::eSetMaximum, unpackedVal);
   }
 
   // Warn the user about the error if we need to.
-  if (internal_ShouldLogError(sr)) {
+  if (sr != ScalarResult::Ok) {
     internal_LogScalarError(aName, sr);
   }
 
-  return MapToNsResult(sr);
+  return NS_OK;
 }
 
 /**
  * Sets the scalar to the maximum of the current and the passed value.
  *
  * @param aId The scalar enum id.
  * @param aValue The numeric value to set the scalar to.
  */
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryScalars.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryScalars.js
@@ -82,37 +82,26 @@ add_task(function* test_keyedSerializati
                KEYED_UINT_SCALAR + "." + expectedOtherKey + " must have the correct value.");
 });
 
 add_task(function* test_nonexistingScalar() {
   const NON_EXISTING_SCALAR = "telemetry.test.non_existing";
 
   Telemetry.clearScalars();
 
-  // Make sure we throw on any operation for non-existing scalars.
-  Assert.throws(() => Telemetry.scalarAdd(NON_EXISTING_SCALAR, 11715),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Adding to a non existing scalar must throw.");
-  Assert.throws(() => Telemetry.scalarSet(NON_EXISTING_SCALAR, 11715),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Setting a non existing scalar must throw.");
-  Assert.throws(() => Telemetry.scalarSetMaximum(NON_EXISTING_SCALAR, 11715),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Setting the maximum of a non existing scalar must throw.");
+  // The JS API must not throw when used incorrectly but rather print
+  // a message to the console.
+  Telemetry.scalarAdd(NON_EXISTING_SCALAR, 11715);
+  Telemetry.scalarSet(NON_EXISTING_SCALAR, 11715);
+  Telemetry.scalarSetMaximum(NON_EXISTING_SCALAR, 11715);
 
-  // Make sure we throw on any operation for non-existing scalars.
-  Assert.throws(() => Telemetry.keyedScalarAdd(NON_EXISTING_SCALAR, "some_key", 11715),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Adding to a non existing keyed scalar must throw.");
-  Assert.throws(() => Telemetry.keyedScalarSet(NON_EXISTING_SCALAR, "some_key", 11715),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Setting a non existing keyed scalar must throw.");
-  Assert.throws(() => Telemetry.keyedScalarSetMaximum(NON_EXISTING_SCALAR, "some_key", 11715),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Setting the maximum of a non keyed existing scalar must throw.");
+  // Make sure we do not throw on any operation for non-existing scalars.
+  Telemetry.keyedScalarAdd(NON_EXISTING_SCALAR, "some_key", 11715);
+  Telemetry.keyedScalarSet(NON_EXISTING_SCALAR, "some_key", 11715);
+  Telemetry.keyedScalarSetMaximum(NON_EXISTING_SCALAR, "some_key", 11715);
 
   // Get a snapshot of the scalars.
   const scalars = getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN);
 
   Assert.ok(!(NON_EXISTING_SCALAR in scalars), "The non existing scalar must not be persisted.");
 
   const keyedScalars =
     getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true);
@@ -192,27 +181,22 @@ add_task(function* test_unsignedIntScala
 
   // Setting or adding a negative number must report an error through
   // the console and drop the change (shouldn't throw).
   Telemetry.scalarAdd(UINT_SCALAR, -5);
   Telemetry.scalarSet(UINT_SCALAR, -5);
   Telemetry.scalarSetMaximum(UINT_SCALAR, -1);
   checkScalar(3);
 
-  // What happens if we try to set a value of a different type?
+  // If we try to set a value of a different type, the JS API should not
+  // throw but rather print a console message.
   Telemetry.scalarSet(UINT_SCALAR, 1);
-  Assert.throws(() => Telemetry.scalarSet(UINT_SCALAR, "unexpected value"),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Setting the scalar to an unexpected value type must throw.");
-  Assert.throws(() => Telemetry.scalarAdd(UINT_SCALAR, "unexpected value"),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Adding an unexpected value type must throw.");
-  Assert.throws(() => Telemetry.scalarSetMaximum(UINT_SCALAR, "unexpected value"),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Setting the scalar to an unexpected value type must throw.");
+  Telemetry.scalarSet(UINT_SCALAR, "unexpected value");
+  Telemetry.scalarAdd(UINT_SCALAR, "unexpected value");
+  Telemetry.scalarSetMaximum(UINT_SCALAR, "unexpected value");
   // The stored value must not be compromised.
   checkScalar(1);
 });
 
 add_task(function* test_stringScalar() {
   let checkExpectedString = (expectedString) => {
     const scalars =
       getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN);
@@ -226,31 +210,21 @@ add_task(function* test_stringScalar() {
   let expected = "test string";
   Telemetry.scalarSet(STRING_SCALAR, expected);
   checkExpectedString(expected);
   expected = "漢語";
   Telemetry.scalarSet(STRING_SCALAR, expected);
   checkExpectedString(expected);
 
   // We have some unsupported operations for strings.
-  Assert.throws(() => Telemetry.scalarAdd(STRING_SCALAR, 1),
-                /NS_ERROR_NOT_AVAILABLE/,
-                "Using an unsupported operation must throw.");
-  Assert.throws(() => Telemetry.scalarAdd(STRING_SCALAR, "string value"),
-                /NS_ERROR_NOT_AVAILABLE/,
-                "Using an unsupported operation must throw.");
-  Assert.throws(() => Telemetry.scalarSetMaximum(STRING_SCALAR, 1),
-                /NS_ERROR_NOT_AVAILABLE/,
-                "Using an unsupported operation must throw.");
-  Assert.throws(() => Telemetry.scalarSetMaximum(STRING_SCALAR, "string value"),
-                /NS_ERROR_NOT_AVAILABLE/,
-                "Using an unsupported operation must throw.");
-  Assert.throws(() => Telemetry.scalarSet(STRING_SCALAR, 1),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "The string scalar must throw if we're not setting a string.");
+  Telemetry.scalarAdd(STRING_SCALAR, 1);
+  Telemetry.scalarAdd(STRING_SCALAR, "string value");
+  Telemetry.scalarSetMaximum(STRING_SCALAR, 1);
+  Telemetry.scalarSetMaximum(STRING_SCALAR, "string value");
+  Telemetry.scalarSet(STRING_SCALAR, 1);
 
   // Try to set the scalar to a string longer than the maximum length limit.
   const LONG_STRING = "browser.qaxfiuosnzmhlg.rpvxicawolhtvmbkpnludhedobxvkjwqyeyvmv";
   Telemetry.scalarSet(STRING_SCALAR, LONG_STRING);
   checkExpectedString(LONG_STRING.substr(0, 50));
 });
 
 add_task(function* test_booleanScalar() {
@@ -276,32 +250,22 @@ add_task(function* test_booleanScalar() 
   checkExpectedBool(true);
   Telemetry.scalarSet(BOOLEAN_SCALAR, 0);
   checkExpectedBool(false);
   Telemetry.scalarSet(BOOLEAN_SCALAR, 1.0);
   checkExpectedBool(true);
   Telemetry.scalarSet(BOOLEAN_SCALAR, 0.0);
   checkExpectedBool(false);
 
-  // Check that unsupported operations for booleans throw.
-  Assert.throws(() => Telemetry.scalarAdd(BOOLEAN_SCALAR, 1),
-                /NS_ERROR_NOT_AVAILABLE/,
-                "Using an unsupported operation must throw.");
-  Assert.throws(() => Telemetry.scalarAdd(BOOLEAN_SCALAR, "string value"),
-                /NS_ERROR_NOT_AVAILABLE/,
-                "Using an unsupported operation must throw.");
-  Assert.throws(() => Telemetry.scalarSetMaximum(BOOLEAN_SCALAR, 1),
-                /NS_ERROR_NOT_AVAILABLE/,
-                "Using an unsupported operation must throw.");
-  Assert.throws(() => Telemetry.scalarSetMaximum(BOOLEAN_SCALAR, "string value"),
-                /NS_ERROR_NOT_AVAILABLE/,
-                "Using an unsupported operation must throw.");
-  Assert.throws(() => Telemetry.scalarSet(BOOLEAN_SCALAR, "true"),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "The boolean scalar must throw if we're not setting a boolean.");
+  // Check that unsupported operations for booleans do not throw.
+  Telemetry.scalarAdd(BOOLEAN_SCALAR, 1);
+  Telemetry.scalarAdd(BOOLEAN_SCALAR, "string value");
+  Telemetry.scalarSetMaximum(BOOLEAN_SCALAR, 1);
+  Telemetry.scalarSetMaximum(BOOLEAN_SCALAR, "string value");
+  Telemetry.scalarSet(BOOLEAN_SCALAR, "true");
 });
 
 add_task(function* test_scalarRecording() {
   const OPTIN_SCALAR = "telemetry.test.release_optin";
   const OPTOUT_SCALAR = "telemetry.test.release_optout";
 
   let checkValue = (scalarName, expectedValue) => {
     const scalars =
@@ -446,22 +410,20 @@ add_task(function* test_keyed_uint() {
     getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true);
 
   for (let k = 0; k < 3; k++) {
     const keyName = KEYS[k];
     Assert.equal(keyedScalars[KEYED_UINT_SCALAR][keyName], expectedValues[k],
                  KEYED_UINT_SCALAR + "." + keyName + " must contain the correct value.");
   }
 
-  // Are we still throwing when doing unsupported things on uint keyed scalars?
+  // Do not throw when doing unsupported things on uint keyed scalars.
   // Just test one single unsupported operation, the other are covered in the plain
   // unsigned scalar test.
-  Assert.throws(() => Telemetry.scalarSet(KEYED_UINT_SCALAR, "new_key", "unexpected value"),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Setting the scalar to an unexpected value type must throw.");
+  Telemetry.keyedScalarSet(KEYED_UINT_SCALAR, "new_key", "unexpected value");
 });
 
 add_task(function* test_keyed_boolean() {
   Telemetry.clearScalars();
 
   const KEYED_BOOLEAN_TYPE = "telemetry.test.keyed_boolean_kind";
   const first_key = "first_key";
   const second_key = "second_key";
@@ -485,44 +447,36 @@ add_task(function* test_keyed_boolean() 
 
   keyedScalars =
     getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true);
   Assert.equal(keyedScalars[KEYED_BOOLEAN_TYPE][first_key], false,
                "The key must contain the expected value.");
   Assert.equal(keyedScalars[KEYED_BOOLEAN_TYPE][second_key], true,
                "The key must contain the expected value.");
 
-  // Are we still throwing when doing unsupported things on a boolean keyed scalars?
+  // Do not throw when doing unsupported things on a boolean keyed scalars.
   // Just test one single unsupported operation, the other are covered in the plain
   // boolean scalar test.
-  Assert.throws(() => Telemetry.keyedScalarAdd(KEYED_BOOLEAN_TYPE, "somehey", 1),
-                /NS_ERROR_NOT_AVAILABLE/,
-                "Using an unsupported operation must throw.");
+  Telemetry.keyedScalarAdd(KEYED_BOOLEAN_TYPE, "somehey", 1);
 });
 
 add_task(function* test_keyed_keys_length() {
   Telemetry.clearScalars();
 
   const LONG_KEY_STRING =
     "browser.qaxfiuosnzmhlg.rpvxicawolhtvmbkpnludhedobxvkjwqyeyvmv.somemoresowereach70chars";
   const NORMAL_KEY = "a_key";
 
   // Set the value for a key within the length limits.
   Telemetry.keyedScalarSet(KEYED_UINT_SCALAR, NORMAL_KEY, 1);
 
-  // Now try to set and modify the value for a very long key.
-  Assert.throws(() => Telemetry.keyedScalarAdd(KEYED_UINT_SCALAR, LONG_KEY_STRING, 10),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Using keys longer than 70 characters must throw.");
-  Assert.throws(() => Telemetry.keyedScalarSet(KEYED_UINT_SCALAR, LONG_KEY_STRING, 1),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Using keys longer than 70 characters must throw.");
-  Assert.throws(() => Telemetry.keyedScalarSetMaximum(KEYED_UINT_SCALAR, LONG_KEY_STRING, 10),
-                /NS_ERROR_ILLEGAL_VALUE/,
-                "Using keys longer than 70 characters must throw.");
+  // Now try to set and modify the value for a very long key (must not throw).
+  Telemetry.keyedScalarAdd(KEYED_UINT_SCALAR, LONG_KEY_STRING, 10);
+  Telemetry.keyedScalarSet(KEYED_UINT_SCALAR, LONG_KEY_STRING, 1);
+  Telemetry.keyedScalarSetMaximum(KEYED_UINT_SCALAR, LONG_KEY_STRING, 10);
 
   // Make sure the key with the right length contains the expected value.
   let keyedScalars =
     getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true);
   Assert.equal(Object.keys(keyedScalars[KEYED_UINT_SCALAR]).length, 1,
                "The keyed scalar must contain exactly 1 key.");
   Assert.ok(NORMAL_KEY in keyedScalars[KEYED_UINT_SCALAR],
             "The keyed scalar must contain the expected key.");
@@ -545,25 +499,19 @@ add_task(function* test_keyed_max_keys()
   let valueToSet = 0;
   keyNamesSet.forEach(keyName => {
     Telemetry.keyedScalarSet(KEYED_UINT_SCALAR, keyName, valueToSet++);
   });
 
   // Perform some operations on the 101th key. This should throw, as
   // we're not allowed to have more than 100 keys.
   const LAST_KEY_NAME = "overflowing_key";
-  Assert.throws(() => Telemetry.keyedScalarAdd(KEYED_UINT_SCALAR, LAST_KEY_NAME, 10),
-                /NS_ERROR_FAILURE/,
-                "Using more than 100 keys must throw.");
-  Assert.throws(() => Telemetry.keyedScalarSet(KEYED_UINT_SCALAR, LAST_KEY_NAME, 1),
-                /NS_ERROR_FAILURE/,
-                "Using more than 100 keys must throw.");
-  Assert.throws(() => Telemetry.keyedScalarSetMaximum(KEYED_UINT_SCALAR, LAST_KEY_NAME, 10),
-                /NS_ERROR_FAILURE/,
-                "Using more than 100 keys must throw.");
+  Telemetry.keyedScalarAdd(KEYED_UINT_SCALAR, LAST_KEY_NAME, 10);
+  Telemetry.keyedScalarSet(KEYED_UINT_SCALAR, LAST_KEY_NAME, 1);
+  Telemetry.keyedScalarSetMaximum(KEYED_UINT_SCALAR, LAST_KEY_NAME, 10);
 
   // Make sure all the keys except the last one are available and have the correct
   // values.
   let keyedScalars =
     getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true);
 
   // Check that the keyed scalar only contain the first 100 keys.
   const reportedKeysSet = new Set(Object.keys(keyedScalars[KEYED_UINT_SCALAR]));