lorenzofox3 / Smart-Table

Code source of Smart Table module: a table/grid for Angularjs
http://lorenzofox3.github.io/smart-table-website/
1.8k stars 513 forks source link

Adjustments for Angular 1.6 (src files) #780

Closed MrWook closed 7 years ago

MrWook commented 7 years ago

Catch timeout promise on cancel because it will throw an error in Angular 1.6

This "error" is referring to the changelog for 1.6.0-rc.0 bracing-vortex (2016-10-26)

c9dffd: report promises with non rejection callback Unhandled rejected promises will be logged to $exceptionHandler

I recreated the error in this plunker It is copy paste from the Pipe example from the documentation. If you click a column header to sort the table too fast, the "$timeout" inside the code will be canceled and with the change for promises it will throw an error. The catch that i added just prevent this.

Thought you can prevent the error for promises if you set $qProvider.errorOnUnhandledRejections(false)inside the config block from the angular module but i think this is not the right approach to disable this globaly for some people.

lorenzofox3 commented 7 years ago

I don't think that is the way to go: the problem comes from the fact that $timeout.cancel(promise) rejects the promise. But If you add a catch handler to the promise, all the (eventually real) errors will be caught and silently ignored which is something you don't want. On the other hand, it seems that if I wrap the func into a function the problem is solved. Check https://plnkr.co/edit/rsRUCd13FqJoB9kQki8z?p=preview (line 381)

MrWook commented 7 years ago

Yes, you are correct didn't think about this issue.

Kind of weird that this work but i guess "func" will return a rejection and pass it to the $timeout. So if you wrap another function around it without a return value it will work. But don't you think this will block some kind of errors too?

lorenzofox3 commented 7 years ago

I am not sure on the why but if you modify func to throw an error: it is not ignored (I have modified the plunk). So it seems to behave as expected: ignore rejection on timeout.cancel while logging error of application code. It might be related to how func is build, I don't know. Do you see the same issue with other directives or only with the stSort ?

MrWook commented 7 years ago

The response from func() looks normaly like this:

{
  "$$state": {
    "status": 1,
    "value": "undefined"
  },
  "$$timeoutId": 18
}

If the func get canceled it will look like this:

{
  "$$state": {
    "pending": "undefined",
    "processScheduled": false,
    "pur": true,
    "status": 2,
    "value": "canceled"
  },
  "$$timeoutId": 18
}

But i don't get this response all the time when it get canceled.

This means that func() itself don't throw the error. The $timeout get the response from func() and will throw the error. So i think it's okey too, that func() will be wrapped inside another function.

I just could recreate this issue for stSort but i think this error can occure for every $timeout.cancel() inside the script. That mean it can occure for 'search' and for the 'pipeline' as well.

This whole "throw an error for a canceled promise" is so weird....

MrWook commented 7 years ago

Hello @lorenzofox3,

should i make a new PR for the change that you suggest or can i change it inside this PR?

lorenzofox3 commented 7 years ago

submit a new one please with the suggestions above, but only for stSort for the time beeing.

Thanks a lot !