More descriptive exception message

[KarlisJ]

trafficstars

We observed a case of getting an error logged (6929293-zd-a8c)

scheduled action 19105 (subscription payment) failed to finish processing due to the following exception: Call to a member function set() on null

which doesn't help debug what is causing the issue.

Changing $e->getMessage() to $e->getTraceAsString() in this line: https://github.com/woocommerce/action-scheduler/blob/0bed905d55cc043ccb6175e66e0d4977a81c173a/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php#L94 gave more descriptive message indicating to the conflicting plugin.

I'm not sure if that change is the best approach but in general the existing reporting does obscure crucial debugging information.

[james-allan]

Changing $e->getMessage() to $e->getTraceAsString() in this line: https://github.com/woocommerce/action-scheduler/blob/0bed905d55cc043ccb6175e66e0d4977a81c173a/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php#L94

I looked into this briefly while submitting https://github.com/woocommerce/action-scheduler/pull/1057 and wanted to add my two cents.

I don't think the suggested approach of creating the exception with the trace as the message is the right approach here. When the exception is created it is passed the previous exception, and so the subsequent code is capable of pulling out the exception message and trace from that previous exception as necessary.

IMO the approach should be either:

1. Update the handle_action_error() function which logs the message to handle pulling the line and trace information from the previous exception. ie $e->getPrevious()->getTraceAsString()
2. Update the call to $this->handle_action_error( $action_id, $e, $context, $valid_action ); (here) to pass the previous exception rather than $e. eg $this->handle_action_error( $action_id, $e->getPrevious(), $context, $valid_action );

I'm not familiar with this section of code to understand the nested try catch to make a judgment on the right way forward here.

[barryhughes]

Sorry for the late response ...

I'm not familiar with this section of code to understand the nested try catch to make a judgment on the right way forward here.

The nested try/catch was mostly to support PHP 5.6 (no support for throwables) alongside PHP 7.0+ (added a concept of throwable errors). We no longer support 5.6, so we may now be able to drastically simplify this and remove the nesting.