Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

lib/core/exception_handler.dart uses wrong log level for exceptions #1706

Open
goderbauer opened this issue Apr 3, 2015 · 3 comments
Open

Comments

@goderbauer
Copy link
Contributor

lib/core/exception_handler.dart uses "print" to log an exception. This will result in a log entry with log level "INFO" in the browser's console. That's not really appropriate for an exception, which should really have log level "ERROR".

This is a problem, because we are checking at the end of each webdriver test, if there are any exceptions present in the browser. For this, we go through the log entries in the console and if we find one that has a log level of ERROR, we mark the test as failed. Unfortunately, due to the bug described above we miss some exceptions because they don't have the appropriate log level. Therefore, some tests are marked as passed even though they really failed and there is no good way of detecting this if exceptions are not logged with the appropriate log level.

@tbosch
Copy link

tbosch commented Apr 3, 2015

Note that simply changing the ExceptionHandler to import dart:html and do a window.console.log does not work on first try, probably because of transformer issues...

@rkirov
Copy link
Contributor

rkirov commented Apr 3, 2015

Given angular.dart is in maintenance mode, I am reluctant to change the default behavior of our exceptionHandler.

Note however that it is trivial to rebind your own exception handler in the tests to perform whatever task makes most sense in the context.

@Injectable()
class RethrowingExceptionHandler {
  call(dynamic error, dynamic stack, [String reason = '']) {
    throw error;
  }
}

main() {
  applicationFactory().addModule(
      new Module()
      ..bind(MyCmp)
      ..bind(ExceptionHandler, toImplementation: RethrowingExceptionHandler)
    ).run();
}

@goderbauer
Copy link
Contributor Author

I am aware of that and that's what we are already doing as a workaround for now. However, we have many people writing many little benchmark apps and it is easy to forget that one bind line. If worse comes to worse, you will never know that you forgot the line because your tests never fail due to this bug.

I feel like the default implementation should at least log the exception correctly as an error.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants