Provide an API to declaratively catch certain errors, bubble others

[machty]

[sukima]

For reference, Ember's PromiseProxyMixin has the exact same problem.

[sukima]

Thoughts on how this might look API wise. Since the intent of ember-concurrency is to leverage as much of the JavaScript language as possible I think a decorator might be the way to go:

import Ember from 'ember';
import { task, didCancel, handledError } from 'ember-concurrency';

export default Ember.Component.extend({
  myTask: task(function * () {
    try {
      yield Ember.RSVP.reject('bork');
    } catch(reason) {
      if (!(didCancel(reason))) {
        throw handledError(reason);
      }
    }
    return 'foobar';
  })
});

In this manor the myTask.last.error will be the reason but it won't bubble to Ember's unhandled rejection code because it was wrapped in handledError(). If it was not wrapped then it would bubble as default.

This would let the coder perform tests on the rejection reason:

if (error instanceof MyExpectedError) {
  throw handledError(err);
} else {
  throw error;
}

rethrowing the handled error means the template can still use myTask.last.error without worry of it reaching Ember's onerror.

[machty]

@sukima recent versions of e-c don't "throw" cancelation errors, so there's no need to use didCancel() here. didCancel() only exists for when you treat a performed task like a promise (you use .then() or .catch()). Since promises don't (yet) have a notion of cancelation, you're forced to explicitly handle cancelation requests in the .catch() handler, which is why you need didCancel. More details on this here

Honestly this issue isn't as problematic as it used to be now that we have the new behavior of cancelation "errors" skipping catch{} blocks. At this point any error handling awkwardness is just as awkward as plain ol JavaScript, and improving the error handling story isn't as high a priority on the ember-concurrency roadmap.

[sukima]

Even with the improved cancellation behavior wouldn't the user still have the problem of differentiating between an expected error (for example a 422 validation errors from a server) and an unexpected one (i.e. programming/runtime error)?

I would like to explicitly mark an exception object as one that is intended to be managed by me (like displayed in the template) and leave the unmarked ones to ember-concurrency who would bubble it up to Ember.onerror so my sentry / logging server can notify me.

Is this something worth looking into? Or should I not add it to my weekend hobby list.

[machty]

Well definitely feel free to explore and experiment, but my hunch is that e-c might not be the library to try and improve the error handling semantics that plague JS in general. On the other hand, if a general purpose pattern catches on in the Ember community in general I'd be happy to bring support to e-c where it makes sense.

[sukima]

I found a possible workaround. In your Ember.onerror function have a trap that checks to see if the exception has a handled property set to true.

Usage

Ember.onerror = function(error) {
  if (Ember.get(error, 'handled')) {
    return; // Do nothing
  }
  // Do your normal error handling
  console.error('Unhandled exception', error);
};

Consumption

myTask: task(function * () {
  try {
    doSomething();
  } catch (error) {
    error.handled = true;
    throw error;
  }
})

And now {{myTask.last.error}} works as expected.

[sukima]

For example I am doing something like this in my app:

locateMe: task(function * () {
  const geoLocation = get(this, 'geoLocation');
  try {
    let result = geoLocation.getPosition();
    set(this, 'boundingBoxValue', this.boundingBoxAround(result));
  } catch (error) {
    if (isGeoLocationError(error)) {
      error.message = geoLocation.translatedMessageForError(error);
      error.handled = true;
    }
    throw error;
  }
}).drop()

[sukima]

For reference I have found a workaround for this issue but does mean the responsibility is pushed to the caller instead of the task itself. I would like to propose a task modifier that does this as part of core.

performTask() {
  let taskInstance = this.get('myTask').perform();
  // Declare my intent to handle the error through myTask.error
  // no-op the default unhandled error handler.
  taskInstance.catch(() => {});
  return taskInstance;
},
myTask: task(function * () {
  yield timeout(1000);
  throw new Error('Bork!');
})

[hrishikeshs]

Hi, I'm facing the same issue. I have a choice between doing

try {
  this.myTask.perform();
} catch(e) {
...
}

versus

this.myTask.perform().catch()

is there any reason to not use this.myTask.perform().catch() ? the documentation on the website almost exclusively uses try..catch

[sukima]

Using try/catch is more expressive and easier to reason about. It also allows the cancellation/error handling to remain in the task. Using .catch() will down cast the TaskInstance to just a Promise and now you have to manage everything manually yourself since you have left safety and comfort of ember-concurrency

[shankarsridhar]

bump. Curious if there are any updates 😃

[hrishikeshs]

I have completely replaced ember-concurrency usage with async/await and am manually managing state with respect to isPending , etc.. I realized that I was not using task cancelation much in my code and wanted to reduce the number of dependencies I was importing.