Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-25219] Android: FileNotFoundExceptions should not be inside the Titanium log #9391

Merged
merged 8 commits into from May 19, 2018

Conversation

sgtcoolguy
Copy link
Contributor

JIRA: https://jira.appcelerator.org/browse/TIMOB-25219

Description:
This removes passing the exception object to the Log.e call, which should avoid printing the full exception stack trace for the exact use case Hans mentioned - when unable to open a file or URL that doesn't exist.

There's some caveats here:

  • It's entirely possible we were unable to load the file/URL input stream for other reasons beyond file not found, and this change would remove the detail to know the underlying cause of failure.
  • It's also pretty likely there are lots of other places in the codebase we try to load files/URLs that we may also log the exception stack trace beyond the one Hans provided in his bug report.

@hansemannn
Copy link
Collaborator

We log the stacktrace in many places (148x). Maybe we want to hide the 3rd parameter in the default logging-state and show it when using trace-logs? How do others solve this? I am very open and happy to discuss it!

@sgtcoolguy sgtcoolguy modified the milestones: 7.0.0, 7.1.0 Nov 30, 2017
@hansemannn hansemannn modified the milestones: 7.1.0, 7.2.0 Feb 26, 2018
@build build added the android label Feb 26, 2018
@hansemannn
Copy link
Collaborator

@sgtcoolguy I think this one is save to merge. Other places can be looked into in further use-cases.

@sgtcoolguy sgtcoolguy modified the milestones: 7.2.0, 7.3.0 May 16, 2018
@lokeshchdhry
Copy link
Contributor

FR Passed.

Studio Ver: 5.1.0.201805150850
SDK Ver: 7.3.0 local build
OS Ver: 10.13.4
Xcode Ver: Xcode 9.3.1
Appc NPM: 4.2.13
Appc CLI: 7.0.3
Daemon Ver: 1.1.1
Ti CLI Ver: 5.1.0
Alloy Ver: 1.12.0
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 10
Devices: ⇨ google Nexus 6P --- Android 8.1.0
⇨ google Nexus 5 --- Android 6.0.1
Emulator: ⇨ Android 4.1.2

@lokeshchdhry
Copy link
Contributor

@sgtcoolguy , the unit tests are still failing with:

11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler: 	it.windowsAndAndroidMissing('.refreshControl (in NavigationWindow)', function (finish) {
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     ^
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler: TypeError: it.windowsAndAndroidMissing is not a function
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     at Suite.<anonymous> (/ti.ui.listview.test.js:872:5)
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     at context.describe.context.context (/ti-mocha.js:1000:9)
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     at /ti.ui.listview.test.js:13:1
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     at Module._runScript (ti:/module.js:613:9)
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     at Module.load (ti:/module.js:105:7)
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     at Module.loadJavascriptText (ti:/module.js:457:9)
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     at Module.loadAsFile (ti:/module.js:512:15)
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     at Module.loadAsFileOrDirectory (ti:/module.js:429:20)
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     at Module.require (ti:/module.js:256:17)
11:45:04 [android unit tests] [ERROR] : �� �TiExceptionHandler:     at require (ti:/module.js:570:15)

Can you please take a look. Thanks !!

@hansemannn
Copy link
Collaborator

@lokeshchdhry Addressed in #10060, but I don't have merge permissions either, so once it turns green, you can merge it.

@build
Copy link
Contributor

build commented May 19, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@hansemannn hansemannn merged commit c04abdb into tidev:master May 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants