Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

web_sys::IdbObjectStore set_name has the wrong return type #4438

Open
chairmank opened this issue Feb 21, 2025 · 3 comments
Open

web_sys::IdbObjectStore set_name has the wrong return type #4438

chairmank opened this issue Feb 21, 2025 · 3 comments
Labels

Comments

@chairmank
Copy link

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 chairmank added the bug label Feb 21, 2025
@chairmank
Copy link
Author

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

@chairmank
Copy link
Author

chairmank commented Feb 21, 2025

Ah, now I see that this may have been a deliberate choice in #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
Copy link
Author

a related issue regarding breaking changes: #4419

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant