Currently, if an exception is thrown when generating a block, a huge horrible fatal error is generated. Really we don't need to have anything quite that exotic. Why don't we put a try/catch around the block generation and not render the block (and emit a regular _drupal_log_error() or something) so that the administrator gets the information, but we don't deliver a full fatal error for every page where the block is to be rendered?

Comments

merlinofchaos’s picture

merlinofchaos commented

Did you really mean this to be in the Views queue?

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

Yes DamZ suggested filing it here since most Commerce blocks are or will be views blocks. I have to confess I filed a core issue as well, so by 2015 this may not be necessary in views.

merlinofchaos’s picture

merlinofchaos commented

Hm. Maybe view execution should try/catch.

dawehner’s picture

dawehner
Primary language German
commented

We catch in query_default at the moment.

merlinofchaos’s picture

merlinofchaos commented

So there really isn't anything we need to do?

dawehner’s picture

dawehner
Primary language German
commented

So it seems to be that drupal commerce throws some exceptions? Is this right?

merlinofchaos’s picture

merlinofchaos commented
Status: Active » Postponed (maintainer needs more info)

Marking NMI. Need to know what might be throwing exceptions we don't already catch.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Postponed (maintainer needs more info) » Active

Actually, Entity module throws a pile of exceptions and both rules and Commerce use it extensively. It's quite fussy and throws a bunch of exceptions like EntityMetadataWrapperException, etc.

@DamZ posted #1184138-01: Catch exceptions for render/entity failures as a workaround for this on pure Commerce blocks. Seems like the approach would work more generally.

The bottom line is, if Views is creating a block and an unhandled exception reaches it, there's no reason for a complete fatal.

merlinofchaos’s picture

merlinofchaos commented
Status: Active » Postponed (maintainer needs more info)

There are a couple of issues here.

1) Why is Views' responsible for catching exceptions thrown by Entity module or Commerce module?
2) Why is a block constructed by Views likely to get these exceptions?

There's no explanations here, Randy, just a "You should do this" that's been repeated a couple of times now. I refuse to add cruft to my module until I understand why my module is responsible for other module's exceptions?

I respect that Views is responsible for catching PDO exceptions because Views is executing the query. That makes sense.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

The idea here is this:

Blocks should not have the power to destroy Drupal installs. There's just no reason to let it be. Views is part of the solution here.

Views is the single best way to implement a block. And it's the single best short-term place to catch this problem.

I think that Drupal core should handle it, see: #1184148: Catch exceptions when building/rendering blocks. But that's a long-term solution, and views could be a shorter-term solution.

esmerel’s picture

esmerel commented
Status: Postponed (maintainer needs more info) » Active
dawehner’s picture

dawehner
Primary language German
commented

Why does the entity module not catch it's own exceptions?

[00:53] <dereine> about this exceptions should this really land in views?
[00:53] <dereine> this seems to be more like babysit broken code
[00:53] <merlinofchaos> dereine: Maybe? I think they're kind of risky at this stage.
[00:53] <merlinofchaos> dereine: Oh, yeah. Those. I don't think we should be catching other module's exceptions.
dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Postponed (maintainer needs more info)

Is that still an issue?

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

I guess I don't see why any block builder should just blindly throw an exception that brings down the site. I'd rather see the fix in core, but still... Why should a block builder encounter an exception and thus render the site inoperable (or at least all pages that contain the block)?

As far as specific Commerce problems, a number of them have been resolved.

esmerel’s picture

esmerel commented
Status: Postponed (maintainer needs more info) » Closed (won't fix)