parallel

parallel copied to clipboard

I'm trying out this package. At the moment my biggest issue with it is the (lack of) debugging support. When a task throws an exception, the original doesn't get it. Instead I only get an instance of TaskException which doesn't even have the message of the original exception, let alone it's type, stack trace and other properties (my custom exceptions often have additional properties for easier debugging).

Ideally I'd like to get the original Exception instance, although I understand that in some cases it might not be serializable. Perhaps we could do some magic to ensure that it is serializable before sending it to the original process?

What are your thoughts on this? I could try to work on a pull request if such feature is desired. For instance something like this could be used as a fallback if plain serialization fails: https://gist.github.com/Thinkscape/805ba8b91cdce6bcaf7c

Exceptions aren't serializable, so we can't provide the original exception. You can use getWorkerTrace() to obtain the original stack trace: https://github.com/amphp/parallel/blob/71be8d1d727c18721a9ba8c2097f46d80a83147f/lib/Worker/TaskException.php#L42-L45

I feel like you didn't even read my post to the end... The point is that trace just as string isn't very useful. And that exceptions mostly are serializable unless there is some unserializable argument somewhere in the stack trace which can be worked around. So we could provide the original (or close to original) exception with some work.

ping @trowski @kelunik I'd like to work on a PR for to improve this but I think it needs some discussion first.

@enumag I pushed a couple commits (4ed05f6aac4eece061ac20a739a7b6834a82e5a5 and 0597620f5605c4874271f2a421545e4ac92dbe80) that improve error messages and serializing the exception trace arguments as you suggested. Please give master a try and let me know what you think.

Many things about the exception such as custom properties are still lost so I wouldn't call it ideal but okay.

The main problem now though is TaskError and TaskException. It's nice that TaskFailure contains improved trace, message and code as properties but that's useless since my code doesn't get TaskFailure - the promise I get from enqueue() is either resolved with the successful value or failed with TaskError or TaskException. I never get an instance of TaskFailure to take advantage of this. So ultimately this only changed the internals but didn't improve anything for me.

Other than that I found one small thing while reviewing your code. See #97.

TaskError and TaskException convey everything available in TaskFailure – the original exception class, message, and code is in the exception message, the previous exception is wrapped in another TaskError/TaskException available through getPrevious(), and getWorkerTrace() returns the flattened exception stack trace (as a string, but still useful for debugging purposes).

Custom properties could be added by using reflection and flattening them in the worker, then adding a method to TaskError/TaskException returning an array of custom properties.

class, message, and code is in the exception message

Which means I'd have to parse the exception message to get them - very bad DX.

as a string, but still useful for debugging purposes

Then what is the purpose of https://github.com/amphp/parallel/commit/0597620f5605c4874271f2a421545e4ac92dbe80?

The intention was for debugging during development where a person would be reading the message. The classes could be extended to make the message, etc. available, but what is your use case where logging the message from TaskError/TaskException is not acceptable?

Then what is the purpose of 0597620?

So entire argument lists were readable in the stack trace. That was what I understood to be your original issue. Having it as a string was for BC, though again the classes could be extended to a make the argument list available as an array.