New issue
Advanced search Search tips

Issue 721412 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Make getUserMedia error names spec compliant

Project Member Reported by guidou@chromium.org, May 11 2017

Issue description

Currently getUserMedia returns error names that do not match the spec.

For example, ConstraintNotSatisfiedError should be 'OverconstrainedError'.

And there are many other examples.
 
code is here: 
https://codesearch.chromium.org/chromium/src/content/renderer/media/user_media_client_impl.cc?q=MediaDeviceNotSupported+package:%5Echromium$&l=1030

PermissionDismissedError => filed https://github.com/w3c/mediacapture-main/issues/451
InvalidStateError => NotReadableError
DevicesNotFoundError => NotFoundError
ConstraintNotSatisfiedError => OverConstrainedError
TrackStartError => NotReadableError
MediaDeviceNotSupported => ? (there used to be a NotSupportedError)
MediaDeviceFailedDueToShutdown => NotReadableError
MediaDeviceKillSwitchOn => NotReadableError

Some of the errors have more detail than NotReadableError. I don't really like losing that information but afaics in my metrics those are quite rare, e.g. MediaDeviceFailedDueToShutdown has an error rate of approximately 1/250.000
most important error:
PermissionDeniedError => NotAllowedError

another thing to keep in mind: it might be possible to implement legacy constraints as "user has not updated", returning the legacy error. This would provide a more gentle transition (for lazy folks).

Comment 4 by fi...@appear.in, Jun 25 2017

hrm... related, how about this one:
  navigator.mediaDevices.getUserMedia({})
  .then(console.log, console.error)

From https://w3c.github.io/mediacapture-main/ this should reject the promise:
    If requestedMediaTypes is the empty set, return a promise rejected with a 
    TypeError.
but it seems to throw and return a resolved promise.

Comment 5 by guidou@chromium.org, Oct 16 2017

Cc: kkaluri@chromium.org guidou@chromium.org
 Issue 746753  has been merged into this issue.

Comment 6 by guidou@chromium.org, Oct 16 2017

Cc: hta@chromium.org orphis@chromium.org
 Issue 645958  has been merged into this issue.

Comment 7 by guidou@chromium.org, Oct 16 2017

Owner: orphis@chromium.org
Status: Assigned (was: Available)

Comment 8 by guidou@chromium.org, Oct 16 2017

Labels: M-64
 Issue 781774  has been merged into this issue.
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/daddca40d184b593e8bd7438029996043ffd3ba7

commit daddca40d184b593e8bd7438029996043ffd3ba7
Author: Florent Castelli <orphis@chromium.org>
Date: Tue Nov 14 12:07:23 2017

Make getUserMedia throw DOMException or OverconstrainedError

The current specification uses DOMException and OverconstrainedError
instead of NavigatorUserMediaError.
Updates many internal errors to map to the proper error name.

Bug:  721412 
Change-Id: I7f3b35bb4435f139af290fd7460396ef2d375f29
Reviewed-on: https://chromium-review.googlesource.com/735324
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Florent Castelli <orphis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516273}
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/chrome/browser/media/webrtc/webrtc_browsertest_base.cc
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/chrome/browser/media/webrtc/webrtc_browsertest_base.h
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/content/browser/webrtc/webrtc_getusermedia_browsertest.cc
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/content/renderer/media/user_media_client_impl_unittest.cc
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/content/renderer/media/user_media_processor.cc
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/content/renderer/media/user_media_processor.h
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/LayoutTests/external/wpt/encrypted-media/encrypted-media-default-feature-policy.https.sub.html
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/LayoutTests/external/wpt/feature-policy/resources/featurepolicy.js
[delete] https://crrev.com/4f1b3023731d6b75dfee59b9514b60df3a42824f/third_party/WebKit/LayoutTests/external/wpt/mediacapture-streams/GUM-impossible-constraint.https-expected.txt
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/LayoutTests/external/wpt/mediacapture-streams/MediaStream-default-feature-policy.https.html
[delete] https://crrev.com/4f1b3023731d6b75dfee59b9514b60df3a42824f/third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-getConstraints-expected.txt
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-getConstraints.html
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/LayoutTests/fast/mediastream/MediaStreamTrack-getSettings.html
[delete] https://crrev.com/4f1b3023731d6b75dfee59b9514b60df3a42824f/third_party/WebKit/LayoutTests/fast/mediastream/getusermedia-constraints-expected.txt
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/LayoutTests/fast/mediastream/getusermedia-constraints.html
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/LayoutTests/fast/mediastream/getusermedia-expected.txt
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/LayoutTests/fast/mediastream/getusermedia-promise.html
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/LayoutTests/fast/mediastream/getusermedia.html
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/LayoutTests/virtual/feature-policy-permissions/external/wpt/mediacapture-streams/GUM-impossible-constraint.https-expected.txt
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/bindings/modules/v8/generated.gni
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/exported/WebUserMediaRequest.cpp
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/mediastream/BUILD.gn
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/mediastream/MediaDevices.cpp
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/mediastream/MediaErrorState.cpp
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/mediastream/MediaErrorState.h
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/mediastream/NavigatorMediaStream.cpp
[delete] https://crrev.com/4f1b3023731d6b75dfee59b9514b60df3a42824f/third_party/WebKit/Source/modules/mediastream/NavigatorUserMediaError.cpp
[delete] https://crrev.com/4f1b3023731d6b75dfee59b9514b60df3a42824f/third_party/WebKit/Source/modules/mediastream/NavigatorUserMediaError.h
[delete] https://crrev.com/4f1b3023731d6b75dfee59b9514b60df3a42824f/third_party/WebKit/Source/modules/mediastream/NavigatorUserMediaError.idl
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/mediastream/NavigatorUserMediaErrorCallback.h
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/mediastream/NavigatorUserMediaErrorCallback.idl
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/mediastream/UserMediaClient.cpp
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/mediastream/UserMediaRequest.cpp
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/mediastream/UserMediaRequest.h
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/Source/modules/modules_idl_files.gni
[modify] https://crrev.com/daddca40d184b593e8bd7438029996043ffd3ba7/third_party/WebKit/public/web/WebUserMediaRequest.h

Comment 11 by hbos@chromium.org, Nov 14 2017

Do we want to write a PSA to discuss-webrtc@ about errors changing, in case any applications are relying on incorrect errors?
#11: Yes, we do.
Do not close this bug until the issue in #4 is fixed.

Comment 14 by fi...@appear.in, Nov 14 2017

can you also double check that https://github.com/webrtc/adapter/blob/master/src/js/chrome/getusermedia.js#L213#-L219 is no longer required ;-)
#11: Do you have any example for such PSA to show me?

#14: I just changed one error type to another, I wouldn't expect any other change.
I think #14 is fixed (at least for the vast majority of cases) since spec-compliant constraints for audio landed in 61.

Comment 17 by fi...@appear.in, Nov 20 2017

guido: yes, #14 looks fixed.

orphis: basically its a "hi, I broke non-spec behaviour you relied on. You are hereby given notice that you have some weeks until this lands in stable. If you don't fix your code before that its your own responsiblity not mine" :-)

In this particular case it might be useful for people to use the error code mapping from https://github.com/webrtc/adapter/blob/master/src/js/chrome/getusermedia.js#L135-L152 so they can look for spec-errors even in chrome stable.

Comment 18 by ai...@tokbox.com, Dec 5 2017

A PSA would be appreciated. We just noticed our tests failing in Chrome Canary.
We just sent the following PSA to webrtc-discuss : https://groups.google.com/forum/#!topic/discuss-webrtc/DMPTKcXVUQ4
Project Member

Comment 20 by bugdroid1@chromium.org, Dec 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f0b15656a06bbe82997540b175ee42b48b994c97

commit f0b15656a06bbe82997540b175ee42b48b994c97
Author: Guido Urdaneta <guidou@chromium.org>
Date: Fri Dec 08 17:11:00 2017

Add a descriptive message to NotFoundError in getUserMedia()

It is currently reporting a default message which is a bit misleading.

Bug:  721412 
Change-Id: I8954eb8b433d70363b099543170d71ec0afb5037
Reviewed-on: https://chromium-review.googlesource.com/817559
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522805}
[modify] https://crrev.com/f0b15656a06bbe82997540b175ee42b48b994c97/content/renderer/media/user_media_processor.cc

Labels: Merge-Request-64 OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
We are requesting merge of r522805 to M64. This CL just adds a descriptive error message that we missed in the original CL.
Labels: -Merge-Request-64 Merge-Approved-64
Approving merge to M64 Chrome OS.
Note that we're planning to go to Beta tomorrow so merging today is appreciated.
Project Member

Comment 24 by bugdroid1@chromium.org, Dec 11 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d7fed5f68e625a2e4822990c1192a8ac9527aba

commit 2d7fed5f68e625a2e4822990c1192a8ac9527aba
Author: Guido Urdaneta <guidou@chromium.org>
Date: Mon Dec 11 16:29:56 2017

Add a descriptive message to NotFoundError in getUserMedia()

It is currently reporting a default message which is a bit misleading.

Bug:  721412 
Change-Id: I8954eb8b433d70363b099543170d71ec0afb5037
Reviewed-on: https://chromium-review.googlesource.com/817559
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522805}(cherry picked from commit f0b15656a06bbe82997540b175ee42b48b994c97)
Reviewed-on: https://chromium-review.googlesource.com/819810
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#129}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/2d7fed5f68e625a2e4822990c1192a8ac9527aba/content/renderer/media/user_media_processor.cc

Status: Fixed (was: Assigned)
#4 is correctly handled and there is a WPT that Chrome passes.

This is enough to close this bug as Fixed.
Cc: jmukthavaram@chromium.org
Labels: Needs-Feedback
guidou@,
Could you please provide us any reproducible steps to check this issue from TE end?
Thanks in advance..!

The way to check is to invoke getUserMedia on a variety of scenarios.
Examples:
-On a machine without any devices (should produce NotFoundError)
-Have the webcam locked by an application other than Chrome (should produce NotReadableError)
-Deny/dismiss permissions (should produce NotAllowedError)

invoking getUserMedia with constraints that cannot be satisfied. For example, getUserMedia({video: {width: {exact: 100000}}}) (should produce OverconstrainedError)

Sign in to add a comment