Unfriendly "Invalid invocation" error without expected signature from custom types in native extension bindings |
|||
Issue descriptionWhen using NativeCrxBindings, the APIs that rely on custom types (ChromeSetting, ContentSetting, StorageArea, EasyUnlockProximityRequired) [1], the error messages are unfriendly error when the API is invoked with invalid parameters. Examples: // Requires "storage" permission. chrome.storage.local.get(); Expected error (with JS bindings): Invocation of form get() doesn't match definition get(optional string or array or object keys, function callback) Actual error with --enable-features=NativeCrxBindings Uncaught TypeError: Invalid invocation // Requires "contentSettings" permission. chrome.contentSettings.javascript.set(); Error with JS bindings (not as descriptive as before, but better than nothing...): Uncaught Error: Parameter 1 (details) is required. Actual error with --enable-features=NativeCrxBindings Uncaught TypeError: Invalid invocation: No matching signature. [1] https://chromium.googlesource.com/chromium/src/+/cadeab771d8ddd5e78d677415b329ad424e693e5/extensions/renderer/native_extension_bindings_system.cc#347
,
Jun 1 2018
,
Jun 4 2018
The NextAction date has arrived: 2018-06-04
,
Jun 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b1c38f6465d5dd12d2e848dde1da790692de9e43 commit b1c38f6465d5dd12d2e848dde1da790692de9e43 Author: Devlin Cronin <rdevlin.cronin@chromium.org> Date: Mon Jun 04 20:48:47 2018 [Extensions Bindings] Provide a better error for StorageArea invocations Provide a better error when a developer invokes a storage area method incorrectly. Add a unittest for the same. Bug: 848904 Change-Id: If3c27d29d35bebb710318333937deecf954c5cbc Reviewed-on: https://chromium-review.googlesource.com/1085486 Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Commit-Queue: Devlin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/master@{#564224} [modify] https://crrev.com/b1c38f6465d5dd12d2e848dde1da790692de9e43/extensions/renderer/storage_area.cc [modify] https://crrev.com/b1c38f6465d5dd12d2e848dde1da790692de9e43/extensions/renderer/storage_area_unittest.cc
,
Jun 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6555759be1d9d4c14ab56c119d47a90b9070b55d commit 6555759be1d9d4c14ab56c119d47a90b9070b55d Author: Devlin Cronin <rdevlin.cronin@chromium.org> Date: Tue Jun 05 15:17:31 2018 [Extensions Bindings] Use better errors for ContentSetting, ChromeSetting invocations Provide a better error when a developer invokes a ContentSetting or ChromeSetting method incorrectly. Add a unittest for the same. Bug: 848904 Change-Id: I0c1a92ea2c4768026f9534e4d0dbe4093ead23f6 Reviewed-on: https://chromium-review.googlesource.com/1086411 Commit-Queue: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Cr-Commit-Position: refs/heads/master@{#564505} [modify] https://crrev.com/6555759be1d9d4c14ab56c119d47a90b9070b55d/chrome/renderer/extensions/custom_types_unittest.cc [modify] https://crrev.com/6555759be1d9d4c14ab56c119d47a90b9070b55d/extensions/renderer/chrome_setting.cc [modify] https://crrev.com/6555759be1d9d4c14ab56c119d47a90b9070b55d/extensions/renderer/content_setting.cc
,
Jun 5 2018
Thanks for the report, Rob! Updated the errors for StorageArea, ChromeSetting, and ContentSetting. From your original examples, the error messages are now: StorageArea: Uncaught TypeError: Error in invocation of storage.get(optional [string|array|object] keys, function callback): No matching signature. Content setting: Uncaught TypeError: Error in invocation of contentSettings.ContentSetting.set(object details, optional function callback): No matching signature. These don't include the signature that was actually passed (e.g., "Invocation of form get() does not..."), but this is in line with the rest of native bindings code. I've been on the fence about how helpful it is to add that, but if we decide to, it should be easy enough to do in one place. (And if you have opinions on that, lemme know.) If we do decide to do that, it can be a separate CL/issue (since this was just about the custom types issues). Closing this one out. |
|||
►
Sign in to add a comment |
|||
Comment 1 by rdevlin....@chromium.org
, Jun 1 2018Owner: rdevlin....@chromium.org
Status: Assigned (was: Untriaged)