Should we cache errorhandlers or not?

[untitaker]

Cont. #1291 (comment)

[untitaker]

cc @flying-sheep @adambyrtek

• If the caching is going to stay, we should document it very well
• Let @mitsuhiko decide?

[adambyrtek]

Short summary of my position:

• When Flask user encounters issues with error handling he is likely to check the contents of error_handler_spec during debugging, and be surprised that there are entries there which haven't been explicitly registered.
• On first sight error_handler_spec seems to be modified only when a new handler is registered. It's not obvious that we pass one of the inner dicts by reference and modify it inside _find_error_handler, especially since the name doesn't suggest that the function has side effects.
• I think optimizations are worth introducing when there is a real performance problem to solve, and I don't see any problem here.

[flying-sheep]

OK, my arguments rehashed:

• The extent of the performance gain is obvious (we save traversing the exception hierarchy upwards every time a certain exception is hit), but it’s not clear if it matters
• The implementation is literally two lines
• There is no cache invalidation since users can’t register new handlers after the app started*
• The two points above mean that there is no potential for failure

The “error_handler_spec will be filled with crap during debugging” argument is good though. we could fix it by using a ChainMap, chaining cache and error_handler_spec. then they are clearly separated, which would also affect the second raised point.

The third point is also valid: We’d need to profile a big flask app with extensive error handler usage, if such a thing exists.

---

*this means that once the first request comes in and might populate the cache, the hierarchy of handled exceptions is frozen, so every exception type will always trigger the same handler in the future of the app lifetime.

[davidism]

Based on #2267, I'm going to remove the cache. I don't expect the hierarchy to be deep enough for this to be meaningful. If there is a performance issue, the developer can always register the handler for the other exceptions manually.

[flying-sheep]

sure, since there seems to be a problem with the cache’s semantixs, go ahead!