New issue
Advanced search Search tips
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
link

Issue 721412: Make getUserMedia error names spec compliant

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

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.
 

Comment 1 by philipp....@googlemail.com, May 12 2017

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

Comment 2 by philipp....@googlemail.com, May 12 2017

most important error:
PermissionDeniedError => NotAllowedError

Comment 3 by philipp....@googlemail.com, May 17 2017

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

Comment 9 by orphis@chromium.org, Nov 6 2017

 Issue 781774  has been merged into this issue.

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

Project Member
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?

Comment 12 by guidou@chromium.org, Nov 14 2017

#11: Yes, we do.

Comment 13 by guidou@chromium.org, Nov 14 2017

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 ;-)

Comment 15 by orphis@chromium.org, Nov 14 2017

#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.

Comment 16 by guidou@chromium.org, Nov 14 2017

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.

Comment 19 by guidou@chromium.org, Dec 5 2017

We just sent the following PSA to webrtc-discuss : https://groups.google.com/forum/#!topic/discuss-webrtc/DMPTKcXVUQ4

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

Project Member
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

Comment 21 by guidou@chromium.org, Dec 11 2017

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.

Comment 22 by kbleicher@chromium.org, Dec 11 2017

Labels: -Merge-Request-64 Merge-Approved-64
Approving merge to M64 Chrome OS.

Comment 23 by kbleicher@chromium.org, Dec 11 2017

Note that we're planning to go to Beta tomorrow so merging today is appreciated.

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

Project Member
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

Comment 25 by guidou@chromium.org, Dec 11 2017

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.

Comment 26 by jmukthavaram@chromium.org, Dec 12 2017

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..!

Comment 27 by guidou@chromium.org, Dec 12 2017

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