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

Add data point for IndexedDB improved error reporting for large value read failures #25277

Conversation

chrisdavidmills
Copy link
Contributor

@chrisdavidmills chrisdavidmills commented Dec 1, 2024

Summary

Chrome has updated its error reporting for large value read failures with transient causes such as low memory and unrecoverable causes such as source blob files being deleted.

Chrome 130 updates the DOMException types to be more appropriate, and Chrome 132 further updates them and improves the associated error messages.

See https://chromestatus.com/feature/5140210640486400 for the data source.

Test results and supporting details

Related issues

@github-actions github-actions bot added data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API size:m [PR only] 25-100 LoC changed labels Dec 1, 2024
@chrisdavidmills chrisdavidmills changed the title Add data point for improved error reporting for large value read fail… Add data point for IndexedDB improved error reporting for large value read fail… Dec 1, 2024
Comment on lines 186 to 188
"improved_reporting_large_value_read_failures": {
"__compat": {
"description": "Improved reporting for large value read failures",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you point me to the part of the spec that mentions this, and/or the spec PR that added this behavior to the spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caugner I'm pretty sure this is the one: w3c/IndexedDB#430.

The exceptions are listed in the spec at https://www.w3.org/TR/IndexedDB/#exceptions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on w3c/IndexedDB#423, I would suggest to rename the feature as follows:

Suggested change
"improved_reporting_large_value_read_failures": {
"__compat": {
"description": "Improved reporting for large value read failures",
"throws_NotReadableError": {
"__compat": {
"description": "Throws `NotReadableError` on storage read failures",

My impression is that "large value" is Chrome terminology, not part of the spec.

/cc @Elchi3 for a second opinion, as we may see more of these very specific behavioral features in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS: We could add this spec_url: https://www.w3.org/TR/IndexedDB/#:~:text=NotReadableError

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like @caugner's description and ID a lot more but I also find it strange to use both, sub features and array support statements here. Is this the summary?

"chrome": [
  {
   "version_added": "132",
   "notes": "Throws `NotReadableError` and `UnknownError`"
  }, 
 {
   "version_added": "130",
   "notes": "Throws `NotFoundError` and `DataError`"
  }, 
 {
   "version_added": "48",
   "notes": "Throws `DOMExceptions`"
  },
  {
   "version_added": "23",
   "notes": "Throws `DOMError`"
  }
],

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From reading the content discussion, I think "Transient and unrecoverable read errors" (transient_unrecoverable_read_errors) is a good feature name and id.

"Improved reporting for read errors" and then only setting Chrome to support and other browsers to false seems wrong given this is about Chrome catching up and not providing "improvements" here, right? If the other browser support it, then we should say so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the feature name as suggested.

I agree it would be nice to include other browser support information, but I've got no idea how to find that out. I can't find a mention of NotReadableError in the Gecko source code so, although they do support the same kind of error handling for read errors, I'm not sure if it is using the same exception names.

From the spec bug, I am wondering whether this has bene agreed as the error to put in the spec, which matches Fx behavior, but Fx hasn't actually implemented the official specified error yet. In which case, could we call it something like "Specified reporting for read errors", and it would be accurate to say that only Chrome has implemented it so far?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Webkit most likely implemented it in https://bugs.webkit.org/show_bug.cgi?id=102137 / WebKit/WebKit@b368491, i.e. Safari 537.43.1 before Safari 7 (537.71).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good enough for me ;-)

Comment on lines 186 to 188
"improved_reporting_large_value_read_failures": {
"__compat": {
"description": "Improved reporting for large value read failures",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on w3c/IndexedDB#423, I would suggest to rename the feature as follows:

Suggested change
"improved_reporting_large_value_read_failures": {
"__compat": {
"description": "Improved reporting for large value read failures",
"throws_NotReadableError": {
"__compat": {
"description": "Throws `NotReadableError` on storage read failures",

My impression is that "large value" is Chrome terminology, not part of the spec.

/cc @Elchi3 for a second opinion, as we may see more of these very specific behavioral features in the future.

{
"version_added": "130",
"partial_implementation": true,
"notes": "Throws a `NotFoundError` for unrecoverable large value read failures, and a `DataError` for transient large value read failures."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the NotFoundError has been in the spec since at least 2011 (before it was referred to as NOT_FOUND_ERR).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true. It is only mentioned here because Chrome reported that error type for read errors for a couple of versions (incorrectly, I assume), before it was then updated to NotReadableError.

api/IDBRequest.json Outdated Show resolved Hide resolved
api/IDBRequest.json Outdated Show resolved Hide resolved
api/IDBRequest.json Outdated Show resolved Hide resolved
api/IDBRequest.json Outdated Show resolved Hide resolved
chrisdavidmills and others added 7 commits December 13, 2024 14:07
Co-authored-by: Claas Augner <[email protected]>
Co-authored-by: Claas Augner <[email protected]>
Co-authored-by: Claas Augner <[email protected]>
Co-authored-by: Florian Scholz <[email protected]>
…lures' of github.com:chrisdavidmills/browser-compat-data into indexeddb-improved-error-reporting-large-value-read-failures
@chrisdavidmills
Copy link
Contributor Author

@caugner @Elchi3 I had to make some further updates due to linting failures. I noticed that the new data point had been put at the wrong nesting level (oops!), and I had to adjust the support versions for FxA and Saf because the specified values were earlier than the support values for the parent feature.

@queengooborg queengooborg changed the title Add data point for IndexedDB improved error reporting for large value read fail… Add data point for IndexedDB improved error reporting for large value read failures Dec 17, 2024
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and plausible to me.

@caugner caugner merged commit d474df5 into mdn:main Dec 20, 2024
9 checks passed
@mdn-bot mdn-bot mentioned this pull request Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API size:m [PR only] 25-100 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants