wasm-bindgen icon indicating copy to clipboard operation
wasm-bindgen copied to clipboard

web_sys::IdbObjectStore set_name has the wrong return type

Open chairmank opened this issue 9 months ago • 3 comments

Describe the Bug

The IndexedDB specification (https://w3c.github.io/IndexedDB/#object-store-interface) and documentation on MDN (https://developer.mozilla.org/en-US/docs/Web/API/IDBObjectStore/name#exceptions) both state that the setter for the name property of IDBObjectStore can throw various exceptions.

Given the pattern of generated method signatures in web_sys, I expect the set_name method of web_sys::IdbObjectStore to return Result<(), JsValue>. Instead, both the Rust compiler and the generated documentation say that the return type of the set_name is ().

Here is the interface description in webidls/enabled/IDB.webidl:

[Exposed=(Window,Worker)]
interface IDBObjectStore {
  [SetterThrows] attribute DOMString name;
  [Throws] readonly attribute any keyPath;
  readonly attribute DOMStringList indexNames;
  [SameObject] readonly attribute IDBTransaction transaction;
  readonly attribute boolean autoIncrement;

Notice the [SetterThrows].

And here is the generated code in src/features/gen_IdbObjectStore.rs:

    # [wasm_bindgen (structural , method , setter , js_class = "IDBObjectStore" , js_name = name)]
    #[doc = "Setter for the `name` field of this object."]
    #[doc = ""]
    #[doc = "[MDN Documentation](https://developer.mozilla.org/en-US/docs/Web/API/IDBObjectStore/name)"]
    #[doc = ""]
    #[doc = "*This API requires the following crate features to be activated: `IdbObjectStore`*"]
    pub fn set_name(this: &IdbObjectStore, value: &str);

Is the #[wasm_bindgen(...)] attribute here missing catch?

Follow up question

There are many other JavaScript web APIs where a property setter can throw. Could there be similar problems elsewhere in web_sys?

chairmank avatar Feb 21 '25 20:02 chairmank

web_sys::IdbIndex has a similar defect in its set_name method.

chairmank avatar Feb 21 '25 20:02 chairmank

Ah, now I see that this may have been a deliberate choice in https://github.com/rustwasm/wasm-bindgen/pull/3998. The BREAKING_SETTER_THROWS constant was added to override code generation, so that existing method return types were preserved for backwards compatibility. The name attributes of IDBObject and IDBIndex are listed here in this constant.

Given that the web-sys crate is still at major version zero, would it make sense to fix these bugs, even at the cost of breaking compatibility?

Alternatively, a conservative approach would be to add #[doc] attributes to acknowledge that these getter/setter methods have the wrong return type and swallow exceptions thrown by JavaScript.

chairmank avatar Feb 21 '25 22:02 chairmank

a related issue regarding breaking changes: https://github.com/rustwasm/wasm-bindgen/issues/4419

chairmank avatar Feb 21 '25 22:02 chairmank