`Thread.errors` not behaving as expected

[ngault]

Uncaught errors in a worker are not communicated to the Thread.errors observable on the main thread. Here is an existing threads.js example on Codesandbox, modified to throw an error in a worker and subscribe to errors on the main thread:

https://codesandbox.io/s/threadsjs-particles-forked-tgqpv?file=/src/worker.js

see line 16 of worker.js for the thrown error
see line 57 of index.js for the Thread.errors observable, which isn't receiving events.

[andywer]

Hey Nick. I think this is expected behavior, but the docs are lacking some information. Thread.errors() will yield errors, but only uncaught worker-wide errors like uncaught promise rejections.

Do you think that fixing the docs will do are you really looking for a way to handle already-handled call errors in a worker-wide manner?

[andywer]

PS: Just to be clear: In your codesandbox example (thanks for providing one!) the error is caught. Errors in exposed methods are always handled by threads.js. We cannot judge from within the library whether you are handling errors of the returned observable/promise in the main thread, however, if you mean that by "uncaught".

[andywer]

PPS: Here is where the Thread.errors() errors originate from:

threads.js/src/master/spawn.ts

Lines 86 to 95 in cf3c1d0

	const rejectionHandler = ((errorEvent: PromiseRejectionEvent) => {
	debugThreadUtils("Unhandled promise rejection event in thread:", errorEvent)
	const workerEvent: WorkerInternalErrorEvent = {
	type: WorkerEventType.internalError,
	error: Error(errorEvent.reason)
	}
	observer.next(workerEvent)
	}) as EventListener
	worker.addEventListener("message", messageHandler)
	worker.addEventListener("unhandledrejection", rejectionHandler)

and

threads.js/src/master/spawn.ts

Lines 120 to 132 in cf3c1d0

	function setPrivateThreadProps<T>(raw: T, worker: WorkerType, workerEvents: Observable<WorkerEvent>, terminate: () => Promise<void>): T & PrivateThreadProps {
	const workerErrors = workerEvents
	.filter(event => event.type === WorkerEventType.internalError)
	.map(errorEvent => (errorEvent as WorkerInternalErrorEvent).error)
	// tslint:disable-next-line prefer-object-spread
	return Object.assign(raw, {
	[$errors]: workerErrors,
	[$events]: workerEvents,
	[$terminate]: terminate,
	[$worker]: worker
	})
	}

[ngault]

Andy - Thanks so much for all this info. Very clear and helpful. I believe there are two problems here. The first is a problem of expectation as to what Thread.errors does. This can be solved by documentation, as you say. But a second, more important, problem remains: namely, threadsjs doesn't appear to provide a way for errors thrown from within exposed functions to be returned via observable to the main thread.

First, let's look at the question of whether Thread.errors should return errors from either, one, both inside exposed methods and outside exposed methods, or, two, only from outside exposed methods. Some level of expectation is reasonably set by Thread.events, which does the former (i.e., reports events from both inside and outside observed methods). For example, subscribing to Thread.events shows events associated with set up and worker termination, in addition to the posted messages from the exposed methods. So, given the current situation, where Thread.events is all inclusive while Thread.errors is not , then, as you suggest, the documentation could be clearer.

Second, if Thread.errors doesn't give access on the main thread to errors thrown within exposed methods, then there should be another way of doing so. But there doesn't appear to be. Using subject.onError in the worker doesn't work. Given that subject.next and subject.complete behave as expected in pushing events to the observable on the main thread, I definitely expected that subject.error would as well. My original expectation was that errors generated by subject.error would be received on both Thread.errors and the values() observables on the main thread. However, neither of these two observables available on the main thread currently can see these errors.

Here's an example, with findings noted in the comments:
https://codesandbox.io/s/cool-microservice-06ut1?file=/src/workers/minmax.js

[ngault]

@andywer , maybe the above was too verbose... is there a way to communicate uncaught errors back to the main thread? Should this not be possible with subject.error? Thanks!

[andywer]

Sorry, thought I already replied to your previous comment.

So Thread.errors() lets you subscribe to uncaught errors in workers, that means any errors thrown outside of exposed functions.

Errors thrown in exposed worker functions are exposed by the observable/promise returned by the function's equivalent in the main thread.

Hope that makes it a bit clearer and that we are not talking about different things here. I think error handling should be documented like this in the future.

[ngault]

Errors thrown in exposed worker functions are exposed by the observable/promise returned by the function's equivalent in the main thread.

@andywer , in the second paragraph of my second post, above, I attempt to explain why this is not true. Please take a look at the sandbox I constructed (same URL as above) that shows the problem exactly:
https://codesandbox.io/s/cool-microservice-06ut1?file=/src/workers/minmax.js

Unless the code in the sandbox is incorrect, that means:

1. The observable on the main thread -- which you've conventionally named values -- does NOT see errors thrown within exposed functions. That's the biggest problem.
2. Threads.errors does NOT report uncaught errors thrown within exposed functions. No problem here.
3. Threads.events does report these same uncaught errors from exposed functions, along with all other events. This seems wrong, especially in view of the way Threads.errors works.

[andywer]

@ngault Ahh, right, now I know where we left the same page!

You didn't have any error handling for the observable in place. Check out my fork of your code sandbox:

https://codesandbox.io/s/vigilant-noyce-wny75 (see file src/index.js)

---

The fact that a pro-user like you struggled with it makes it all the more clear that we need an errror handling section in the docs, though.

Or even better: Is there any way to make it more intuitive? I thought the native error handling functionality of observables would do fine, but maybe it's too unintuitive after all. Maybe it's fine if we just add an example to the docs?

[ngault]

Thanks for your patience with me, I'm hardly an expert here! It makes sense that providing an observer object, as opposed to an argument for a single value, would give access to the error.

What confused me is that a subscribe to Thread.events works differently insofar as errors are treated like all other events, with the error object (and trace) returned on the event message. If I hadn't studied Thread.events and Thread.errors, I probably would have provided an observer object, instead of doing what the examples do in providing a generic argument.

Finally, "yes!" to your suggestion of adding a complete example of error-handling for those of us using observables. Threads.js excels at advanced use of web workers, where handling errors thrown both outside and inside "exposed" methods is a requirement, not an option. So, an example would show the combined use on the main thread of two observables: one, Thread.errors and, two, another observable for "exposed" methods (a.k.a. values() in the docs). This seems to be the only observable-based pattern for catching all errors.

Thanks