FirebaseExtended / firebase-queue

MIT License
786 stars 108 forks source link

Queue does not throw an error if it fails to connect to the provided reference. #62

Open startswithaj opened 8 years ago

startswithaj commented 8 years ago

If you pass in a firebase ref that for instance doesn't have the correct read permissions when instantiating a Queue it does not throw it just logs the error? There's really no way of responding to this.

https://github.com/firebase/firebase-queue/blob/master/src/lib/queue_worker.js#L598

I can't see a way to tell programmatically if a queue worker subscription has been successfully established.

Should there be a function like Queue.shutdown such as Queue.status() that returns a promise that is resolved once all workers successful subscription to the ref has been made or rejected if it fails?

I realise the QueueWorker would have to have a status function that returns its connection state as well.

A less drastic change would be that the workers are not spawn until the ref connection is tested.

I'm happy to take a crack at a PR for this if you guys have suggestions?

A breaking change suggestion would be that Queue has a function called connect() or spawn(). That you call once you instantiate the object.

Cheers

startswithaj commented 8 years ago

A couple of other things that I'm not sure how to handle correctly...

If an error is thrown inside the processing function a worker is running (object undefined etc), it's heavily guarded (which is what you want in a worker) and the error is written to the task. But there is no logging.

In production most of these types of unhandled errors shouldn't happen but it makes things slightly difficult for dev in the case where an error get thrown inside of that processing function you have to track down the errored/retired task and check the error details.

I feel like there should be an option to log out errorDetails inside the worker?

The solution is wrapping the worker code in a try/catch/promise logging the error and then throwing it again.