IndexedDB: Make DOMException messages more useful.

Source/modules/indexeddb/IDBDatabase.cpp

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 30 matching lines @@
 #include "modules/indexeddb/IDBKeyPath.h"
 #include "modules/indexeddb/IDBObjectStore.h"
 #include "modules/indexeddb/IDBTracing.h"
 #include "modules/indexeddb/IDBTransaction.h"
 #include "modules/indexeddb/IDBVersionChangeEvent.h"
 #include <limits>
 #include "wtf/Atomics.h"
 
 namespace WebCore {
 
-const char IDBDatabase::notFoundErrorMessage[] = "An operation failed because the requested database object could not be found.";
+const char IDBDatabase::indexDeletedErrorMessage[] = "The index or its object store has been deleted.";
+const char IDBDatabase::isKeyCursorErrorMessage[] = "The cursor is a key cursor.";
+const char IDBDatabase::noKeyOrKeyRangeErrorMessage[] = "No key or key range specified.";
+const char IDBDatabase::noSuchIndexErrorMessage[] = "The specified index was not be found.";
+const char IDBDatabase::noSuchObjectStoreErrorMessage[] = "The specified object store was not found.";
+const char IDBDatabase::notGotValueErrorMessage[] = "The cursor is being iterated or has iterated past its end.";
+const char IDBDatabase::notValidKeyErrorMessage[] = "The parameter is not a valid key.";
+const char IDBDatabase::notVersionChangeTransactionErrorMessage[] = "The database is not running a version change transaction.";
+const char IDBDatabase::objectStoreDeletedErrorMessage[] = "The object store has been deleted.";
+const char IDBDatabase::requestNotFinishedErrorMessage[] = "The request has not finished.";
+const char IDBDatabase::sourceDeletedErrorMessage[] = "The cursor's source or effective object store has been deleted.";
+const char IDBDatabase::transactionFinishedErrorMessage[] = "The transaction has finished.";
 
 PassRefPtr<IDBDatabase> IDBDatabase::create(ScriptExecutionContext* context, PassRefPtr<IDBDatabaseBackendInterface> database, PassRefPtr<IDBDatabaseCallbacks> callbacks)
 {
     RefPtr<IDBDatabase> idbDatabase(adoptRef(new IDBDatabase(context, database, callbacks)));
     idbDatabase->suspendIfNeeded();
     return idbDatabase.release();
 }
 
 IDBDatabase::IDBDatabase(ScriptExecutionContext* context, PassRefPtr<IDBDatabaseBackendInterface> backend, PassRefPtr<IDBDatabaseCallbacks> callbacks)
     : ActiveDOMObject(context)
@@ skipping 107 matching lines @@
     }
 
     return createObjectStore(name, keyPath, autoIncrement, es);
 }
 
 PassRefPtr<IDBObjectStore> IDBDatabase::createObjectStore(const String& name, const IDBKeyPath& keyPath, bool autoIncrement, ExceptionState& es)
 {
     IDB_TRACE("IDBDatabase::createObjectStore");
     HistogramSupport::histogramEnumeration("WebCore.IndexedDB.FrontEndAPICalls", IDBCreateObjectStoreCall, IDBMethodsMax);
     if (!m_versionChangeTransaction) {
-        es.throwDOMException(InvalidStateError);
+        es.throwDOMException(InvalidStateError, IDBDatabase::notVersionChangeTransactionErrorMessage);
+        return 0;
+    }
+    if (m_versionChangeTransaction->isFinished()) {
+        es.throwDOMException(TransactionInactiveError, IDBDatabase::transactionFinishedErrorMessage);
         return 0;
     }
     if (!m_versionChangeTransaction->isActive()) {
         es.throwDOMException(TransactionInactiveError);
         return 0;
     }
 
     if (containsObjectStore(name)) {
-        es.throwDOMException(ConstraintError);
+        es.throwDOMException(ConstraintError, "An object store with the specified name already exists.");
         return 0;
     }
 
     if (!keyPath.isNull() && !keyPath.isValid()) {
-        es.throwDOMException(SyntaxError);
+        es.throwDOMException(SyntaxError, "The keyPath option is not a valid key path.");

[dgrogan, 2013/07/12 17:58:17]

What do you think of splitting this up and including the invalid keypath in the
error message? Though I see that throwDOMException only takes a const char*. Why
is that?

[arv (Not doing code reviews), 2013/07/12 18:07:29]

One step at a time. You can use String.format or we can extend ExceptionState to
support other versions.

[jsbell, 2013/07/12 18:22:19]

Yeah, we could go further and include the details (the key path, the not
found/duplicate name, etc). In the case of key path it's slightly tricky as we'd
potentially need to serialize the array.

         return 0;
     }
 
     if (autoIncrement && ((keyPath.type() == IDBKeyPath::StringType && keyPath.string().isEmpty()) || keyPath.type() == IDBKeyPath::ArrayType)) {
-        es.throwDOMException(InvalidAccessError);
+        es.throwDOMException(InvalidAccessError, "The autoIncrement option was set but the keyPath option was not a non-empty string.");
         return 0;
     }
 
     int64_t objectStoreId = m_metadata.maxObjectStoreId + 1;
     m_backend->createObjectStore(m_versionChangeTransaction->id(), objectStoreId, name, keyPath, autoIncrement);
 
     IDBObjectStoreMetadata metadata(name, objectStoreId, keyPath, autoIncrement, IDBDatabaseBackendInterface::MinimumIndexId);
     RefPtr<IDBObjectStore> objectStore = IDBObjectStore::create(metadata, m_versionChangeTransaction.get());
     m_metadata.objectStores.set(metadata.id, metadata);
     ++m_metadata.maxObjectStoreId;
 
     m_versionChangeTransaction->objectStoreCreated(name, objectStore);
     return objectStore.release();
 }
 
 void IDBDatabase::deleteObjectStore(const String& name, ExceptionState& es)
 {
     IDB_TRACE("IDBDatabase::deleteObjectStore");
     HistogramSupport::histogramEnumeration("WebCore.IndexedDB.FrontEndAPICalls", IDBDeleteObjectStoreCall, IDBMethodsMax);
     if (!m_versionChangeTransaction) {
-        es.throwDOMException(InvalidStateError);
+        es.throwDOMException(InvalidStateError, IDBDatabase::notVersionChangeTransactionErrorMessage);
+        return;
+    }
+    if (m_versionChangeTransaction->isFinished()) {
+        es.throwDOMException(TransactionInactiveError, IDBDatabase::transactionFinishedErrorMessage);
         return;
     }
     if (!m_versionChangeTransaction->isActive()) {
         es.throwDOMException(TransactionInactiveError);

[dgrogan, 2013/07/12 17:58:17]

Was this skipped intentionally?

[jsbell, 2013/07/12 18:22:19]

I was thinking the generic message was adequate, but it would be better to
remove the "or which is finished." from it.

         return;
     }
 
     int64_t objectStoreId = findObjectStoreId(name);
     if (objectStoreId == IDBObjectStoreMetadata::InvalidId) {
-        es.throwDOMException(NotFoundError, IDBDatabase::notFoundErrorMessage);
+        es.throwDOMException(NotFoundError, "The specified object store was not found.");
         return;
     }
 
     m_backend->deleteObjectStore(m_versionChangeTransaction->id(), objectStoreId);
     m_versionChangeTransaction->objectStoreDeleted(name);
     m_metadata.objectStores.remove(objectStoreId);
 }
 
 PassRefPtr<IDBTransaction> IDBDatabase::transaction(ScriptExecutionContext* context, const Vector<String>& scope, const String& modeString, ExceptionState& es)
 {
     IDB_TRACE("IDBDatabase::transaction");
     HistogramSupport::histogramEnumeration("WebCore.IndexedDB.FrontEndAPICalls", IDBTransactionCall, IDBMethodsMax);
     if (!scope.size()) {
-        es.throwDOMException(InvalidAccessError);
+        es.throwDOMException(InvalidAccessError, "The storeNames parameter was empty.");
         return 0;
     }
 
     IndexedDB::TransactionMode mode = IDBTransaction::stringToMode(modeString, es);
     if (es.hadException())
         return 0;
 
-    if (m_versionChangeTransaction || m_closePending) {
-        es.throwDOMException(InvalidStateError);
+    if (m_versionChangeTransaction) {
+        es.throwDOMException(InvalidStateError, "A version change transaction is running.");
         return 0;
     }
 
+    if (m_closePending) {
+        es.throwDOMException(InvalidStateError, "The database connection is closing.");
+        return 0;
+    }
+
     Vector<int64_t> objectStoreIds;
     for (size_t i = 0; i < scope.size(); ++i) {
         int64_t objectStoreId = findObjectStoreId(scope[i]);
         if (objectStoreId == IDBObjectStoreMetadata::InvalidId) {
-            es.throwDOMException(NotFoundError, IDBDatabase::notFoundErrorMessage);
+            es.throwDOMException(NotFoundError, "One of the specified object stores was not found.");
             return 0;
         }
         objectStoreIds.append(objectStoreId);
     }
 
     int64_t transactionId = nextTransactionId();
     m_backend->createTransaction(transactionId, m_databaseCallbacks, objectStoreIds, mode);
 
     RefPtr<IDBTransaction> transaction = IDBTransaction::create(context, transactionId, scope, mode, this);
     return transaction.release();
@@ skipping 121 matching lines @@
 {
     return &m_eventTargetData;
 }
 
 EventTargetData* IDBDatabase::ensureEventTargetData()
 {
     return &m_eventTargetData;
 }
 
 } // namespace WebCore