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

Bikeshedding help needed for generic over-limit error #1463

Open
domenic opened this issue Jan 24, 2025 · 9 comments · May be fixed by #1465
Open

Bikeshedding help needed for generic over-limit error #1463

domenic opened this issue Jan 24, 2025 · 9 comments · May be fixed by #1465

Comments

@domenic
Copy link
Member

domenic commented Jan 24, 2025

What is the issue with the Web IDL Standard?

In webmachinelearning/writing-assistance-apis#31 I proposed adding a TooManyTokens DOMException-derived interface which gives information on how far over the token limit you are. It had properties tokenCount and tokensAvailable, such that ex.tokenCount > ex.tokenAvailable is the error condition being signaled.

@michaelwasserman pointed out that this would probably be worth generalizing beyond "tokens", for the entire web ecosystem to use. I tend to agree. I'm just unsure what the general names should be.

My current best proposal is: QuotaExceededDetailedError, with properties quota and request. This plays off of the existing, non-detailed "QuotaExceededError" DOMException.

(But, maybe "quota" feels too storage-specific? Check out "QuotaExceededError" DOMException's existing usages on the web to see what you think.)

Other options:

  • LimitExceededError, LimitError, OverageError
  • requested (instead of request), limit, max, available
  • amount, value, attempted

Does the community have any thoughts on what would be the best names, which are suitable for the web API ecosystem as a whole?

@jasnell
Copy link

jasnell commented Jan 24, 2025

New custom error types like this tend to be difficult to implement in practice. I'd almost prefer for DOMException to allow for an arbitrary details own property that could express additional detail like this.

If not that, something like LimitExceededError might work?

@domenic
Copy link
Member Author

domenic commented Jan 24, 2025

New custom error types are not difficult for the web, at least. Whereas arbitrary properties are much more difficult, especially given issues around getters that return dictionaries.

Anyway, can you say more about your preference for the LimitExceededError option over the other options I listed?

@bakkot
Copy link
Contributor

bakkot commented Jan 24, 2025

Given that QuotaExceededError is already used for "requested too many bytes" in crypto (sidebar: why do we have this limit) and "requested too many recognizers" in the Handwriting Recognition API, I think it's already been generalized beyond just storage, so it seems fine to continue using it for other kinds of quota.

Does it actually need to be a new type? Is there a reason not to just add more properties to the existing QuotaExceededError type? The usual thing in JS would be to have optional properties for when additional data is available (like how errors now all have an optional cause), rather than having different types.

@jasnell
Copy link

jasnell commented Jan 24, 2025

@bakkot ... Good point on QuotaExceeded already being used in other contexts. Given that I think I can live with it.

@domenic ... My preference for LimitExceeded is really just subjective. I just happen to like that shade of paint for the bike shed. But again, given a choice it would be a shame to have to introduce a whole new error type.

@domenic
Copy link
Member Author

domenic commented Jan 24, 2025

Does it actually need to be a new type? Is there a reason not to just add more properties to the existing QuotaExceededError type? The usual thing in JS would be to have optional properties for when additional data is available (like how errors now all have an optional cause), rather than having different types.

The problem is that "QuotaExceededError" is just the value of the name property for certain DOMExceptions, due to DOMException's unfortunate design. (Which mismatches the base JS language types). It doesn't have its own distinct type.

So if we want to add properties, our options are:

  1. New subclass of DOMException (my proposal)
  2. Conditionally add data properties or getters to DOMException instances (maybe this is what you're suggesting?)
  3. New error type that isn't a subclass of DOMException
  4. Modify DOMException and have the getters return undefined most of the time

(2) and (3) require custom bindings, which web browser implementations are generally loathe to do. So we created the (1) path, with first-class support in the spec, since it seemed like the best we could do, and it paved a cowpath that at least two specs were already following.

If implementers think (2) is reasonable, maybe we could do that instead. I agree that not having two different types of "quota exceeded error"s in the platform is valuable. I've pinged our Blink bindings expert, @natechapin, to get his take on how disruptive it would be to implement (2).

@bakkot
Copy link
Contributor

bakkot commented Jan 24, 2025

Ah, I'd forgotten that's how it works. No, I wouldn't be in favor of conditionally adding properties to base DOMException instances. (I mean, it's not a terrible option, but it wouldn't be my preferred route).

What about doing (1), but calling it QuotaExceededError, and retrofitting the existing specs to use the new subclass instead of the base type? It's hard to imagine this actually breaking anything, since it would still be a subclass of DOMException and the .name would still match.

@whatwg whatwg deleted a comment Jan 24, 2025
@domenic
Copy link
Member Author

domenic commented Jan 24, 2025

It's a good point that instanceof checks would still work. .constructor and .constructor.name checks would break, but hopefully everyone is doing .name checks...

It might be a slightly hard sell to take on compat risk, but maybe it's worth trying.

@bakkot
Copy link
Contributor

bakkot commented Jan 24, 2025

It doesn't look like very many people are checking the .constructor of DOMExceptions (not zero people, but not many). And QuotaExceededError is also one of the more obscure errors; very few pages are going to run into it at all. So I'm hopeful it would be web-compat.

@domenic
Copy link
Member Author

domenic commented Jan 27, 2025

I've put up a proposal for upgrading QuotaExceededError at #1465. Thanks @bakkot for the push.

I haven't yet discussed it within Chromium to see if we're ready to officially stand behind it, but at least people can see what it looks like. I personally think it's a good idea and will report back.

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

Successfully merging a pull request may close this issue.

3 participants