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

Request for design principle clarification: Warn explicitly against getters throwing exceptions #400

Closed
wolenetz opened this issue Oct 3, 2022 · 4 comments

Comments

@wolenetz
Copy link
Contributor

wolenetz commented Oct 3, 2022

The design principles for JavaScript attributes currently says:

It would be good to verbosely clarify that exceptions are a form of a side effect: attribute getters should not throw exceptions.

Unfortunately, attribute getters throwing exceptions is an antipattern existing in many web APIs already. So perhaps this principle should be applied only for the following (or a subset of the following):

  • changes to existing APIs, such as adding new attributes to existing API objects, or
  • adding new APIs containing objects with attribute getters.

An example where this policy clarification would have been helpful was my recent attempt to launch "MSE-in-Workers" in Chrome 105 following an MSE specification update for the feature that changed a new "getHandle()" method to instead be a "[SameObject] readonly attribute handle" attribute:

  • The previously proposed "getHandle()" method threw a NotSupportedError exception if it were called on a context (e.g., main/Window thread, not Dedicated Worker thread) where the implementation did not yet support creation of a MediaSourceHandle for any MediaSource objects owned by that context. In spec review, that method was changed to be a readonly attribute getter, but it retained the ability to throw NotSupportedError exception.
  • While the spec change for this landed without concern for exception throwing by the attribute getter, the Chromium change received a review comment questioning this. I searched both the Chromium implementation for pre-existing examples of attribute getters throwing exceptions (and found many) and consulted the design principles - but search for "exception" didn't get a quick hit.
  • Unaware that this addition to the pre-existing main/Window context exposure of the MediaSource API, when used by at least one older version of an MSE library (video.js) that cloned MediaSource object instances and didn't expect exception, the feature was shipped on-by-default in Chrome 105. Once that issue was detected, it was unlaunched and has only recently been relaunched (as of Chrome 108.0.5334.0) after both the spec and implementation were updated to avoid this scenario.
@cynthia
Copy link
Member

cynthia commented Nov 8, 2022

Thanks for raising this, would you be interested in sending us a PR to address this? We could probably just call it out explicitly in 6.3.

@torgo torgo modified the milestones: 2022-11-07-week, 2022-12-05-week Dec 4, 2022
@torgo
Copy link
Member

torgo commented Dec 4, 2022

Hi @wolenetz just a gentle ping on this one. We're going to be working on Design Principles in the coming week so if you feel motivated to write us a PR on this one, it would be great timing. :)

@wolenetz
Copy link
Contributor Author

Sure, I'll prep a PR shortly on this. While it may not apply in every situation, especially pre-existing APIs that already throw exceptions on getters, it may help prevent regressing existing non-throwing getters in future.

wolenetz added a commit to wolenetz/design-principles that referenced this issue Dec 20, 2022
@wolenetz
Copy link
Contributor Author

Please review #408 as potential fix for this issue.

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

No branches or pull requests

5 participants