Runtime debug statement in Views now prints out object

I would say, take it out. Printing debug information on a live system feels wrong. Looks like Views is the only place in core where this currently happens outside of tests, so should we maybe take them all out?

Other spots I see:
\Drupal\views\Plugin\views\field\FieldPluginBase::addAdditionalFields
\Drupal\views\Plugin\views\style\Opml::render
\Drupal\views\Plugin\views\style\Rss::render
\Drupal\views\Plugin\views\style\StylePluginBase::render

Patch is just for taking out the one in \Drupal\views\ViewExecutable::setDisplay()

+++ b/core/modules/views/src/ViewExecutable.php
@@ -788,7 +788,6 @@ public function setDisplay($display_id = NULL) {
-      debug(format_string('setDisplay() called with invalid display ID "@display".', array('@display' => $display_id)));

+++ b/core/modules/views/tests/src/Kernel/ViewExecutableTest.php
@@ -195,8 +195,6 @@ public function testSetDisplayWithInvalidDisplay() {
-    // Error is triggered while calling the wrong display.
-    $this->setExpectedException(\PHPUnit_Framework_Error::class);

Can we maybe use a trigger_error or at least some error logging instead?

I agree that some sort of error logging would be more helpful than suppressing/not showing the error at all.

Needs work based upon #6 and #5

Well definitely v2 for me.

Just thinking about this, and why do we tolerate an invalid ID in the first place? Fail early, fail hard. This is not something that should ever come up in normal use/through the UI, so it's people using bad code, so lets make sure it fails.

This might be to much of an API change, but throwing an exception makes more sense to me. Lets see if this makes anything else fail.

This code is probably coming from Drupal 6, where exceptions weren't really a thing, given views was compatible with php 4.

On the other hand, its a bit tough for me to judge, whether its okay or not to potentially break sites like that. Currently they render the default display, don't they? I think using trigger_error for a while would be certainly the more safe option.

Yeah, from a BC standpoint the exception is too much for now, just wanted to put it out into the world.

Just re-upping @vaplas' patch from #8 to make the last patch in the issue the one we are discussing. I think we all agree that trigger_error is the soft but right way to go here right? So RTBC then.

Bot happy, we happy.

The exception vs. error decision is not one I'm comfortable making but the patch looks great to me. Adding credit for @tinny as mentioned in the OP.

Agreed on trigger_error() vs. introducing a new exception here, however do we want to make that an E_USER_WARNING instead of a notice, that might help to clean up existing incidences of the bug more so that we can introduce an exception in 9.0.0

Also - should we open a follow-up to introduce the exception?

Nice.

Should we have a follow up to deal with the other debug statements in Views? IMO debug statements should not live in core code and as @dawehner pointed out in #11, this is probably a leftover from D6/PHP4 times, so it could do with an update to more current best practice.

Part of this patch got into #2897009: MediaInterface is missing setName() and getName() so that is why this no longer applies.

#2897009: MediaInterface is missing setName() and getName() got fixed by @webchick, this applies again, so back to RTBC

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!