JSErrorCount doesn't count errors that happen too early in the page |
||||
Issue descriptionCurrently JSErrorCount is loaded after several scripts have been included so error that happens during these initial scripts aren't accounted.
,
Oct 9
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0ab71fd030430ef7548b737936e5896b2f2381ba commit 0ab71fd030430ef7548b737936e5896b2f2381ba Author: Luciano Pacheco <lucmult@chromium.org> Date: Tue Oct 09 09:20:58 2018 Fix JSErrorCount for promises and move it before all scripts Change error_util.js include to be before all scripts to be able to catch errors of any scripts included. Add event listener for "unhandledrejection" event to be able to detect unhandled errors in Promises. Bug: 893430 Change-Id: I920f5ab559c4b0789e96047313ea5f8a394bad44 Reviewed-on: https://chromium-review.googlesource.com/c/1270676 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Cr-Commit-Position: refs/heads/master@{#597855} [modify] https://crrev.com/0ab71fd030430ef7548b737936e5896b2f2381ba/ui/file_manager/file_manager/common/js/error_util.js [modify] https://crrev.com/0ab71fd030430ef7548b737936e5896b2f2381ba/ui/file_manager/file_manager/foreground/js/main_scripts.js
,
Oct 10
Next step here is to fix in the other apps too, actually they should share the same JSError functions which they dont' atm. :-/
,
Oct 10
The js_unit_tests gn template has a `mocks` variable
# Variables:
# deps:
# List of js_library targets to depend on
#
# mocks:
# An optional list of .js files to load before any other scripts
It's currently just used by image_loader:
js_unit_tests("unit_tests") {
deps = [
":exif_encoder_unittest",
":image_encoder_unittest",
":image_view_unittest",
]
mocks = [ "../../../file_manager/foreground/js/metadata/metadata_dispatcher_mock_deps.js" ]
}
(that .js is just
"""
// importScripts is used in metadata_dispatcher.js which is designed to work
// inside SharedWorker.
function importScripts(arg1) {}
"""
(nit: we should probably rename/consolidate the multiple error_util.js, stop mentioning `SelectFileDialogExtensionBrowserTest` specifically, and rename error_util.js --> js_error_counter.js or something)
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8510ab3abb7e58e335c1d23cb41f1d7b7c702f1c commit 8510ab3abb7e58e335c1d23cb41f1d7b7c702f1c Author: Luciano Pacheco <lucmult@chromium.org> Date: Tue Oct 16 05:21:38 2018 Move error_util.js to base/ Move library to base to be able to share it with other apps. Fix the comment about metrics.js to be on top of metrics.js include. Move error_util.js in background page to be first script loaded to be able to catch errors from all scripts. Bug: 893430 Change-Id: I9744089d258fc36979e88b1418595f733e00b81f Cq-Include-Trybots: luci.chromium.try:closure_compilation Reviewed-on: https://chromium-review.googlesource.com/c/1282642 Reviewed-by: Noel Gordon <noel@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#599864} [modify] https://crrev.com/8510ab3abb7e58e335c1d23cb41f1d7b7c702f1c/ui/file_manager/base/js/BUILD.gn [rename] https://crrev.com/8510ab3abb7e58e335c1d23cb41f1d7b7c702f1c/ui/file_manager/base/js/error_util.js [modify] https://crrev.com/8510ab3abb7e58e335c1d23cb41f1d7b7c702f1c/ui/file_manager/file_manager/background/js/BUILD.gn [modify] https://crrev.com/8510ab3abb7e58e335c1d23cb41f1d7b7c702f1c/ui/file_manager/file_manager/background/js/background_scripts.js [modify] https://crrev.com/8510ab3abb7e58e335c1d23cb41f1d7b7c702f1c/ui/file_manager/file_manager/common/js/BUILD.gn [modify] https://crrev.com/8510ab3abb7e58e335c1d23cb41f1d7b7c702f1c/ui/file_manager/file_manager/foreground/js/main_scripts.js
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a8dfb8e9ae0112c506e35cacff4389b8a5690374 commit a8dfb8e9ae0112c506e35cacff4389b8a5690374 Author: Luciano Pacheco <lucmult@chromium.org> Date: Tue Oct 16 08:52:09 2018 Rename error_util to error_counter Rename error_util.js to error_counter.js to better describe its usage and avoid the generic term "util". Bug: 893430 Change-Id: I9cbb7f2f5ee09bf02d190dfb27d05e762bdde59f Cq-Include-Trybots: luci.chromium.try:closure_compilation Reviewed-on: https://chromium-review.googlesource.com/c/1282682 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#599921} [modify] https://crrev.com/a8dfb8e9ae0112c506e35cacff4389b8a5690374/ui/file_manager/base/js/BUILD.gn [rename] https://crrev.com/a8dfb8e9ae0112c506e35cacff4389b8a5690374/ui/file_manager/base/js/error_counter.js [modify] https://crrev.com/a8dfb8e9ae0112c506e35cacff4389b8a5690374/ui/file_manager/file_manager/background/js/BUILD.gn [modify] https://crrev.com/a8dfb8e9ae0112c506e35cacff4389b8a5690374/ui/file_manager/file_manager/background/js/background_scripts.js [modify] https://crrev.com/a8dfb8e9ae0112c506e35cacff4389b8a5690374/ui/file_manager/file_manager/foreground/js/main_scripts.js
,
Oct 17
This is the final stage that I'm working to get to, see screenshot it will show: 1. a specific prefix "[unhandled-erro]:", so it's easier to identify among other logs. 2. in developer tools, stack trace that led to the console.error() or console.assert() so we can click on the caller site and see where it happened. Unfortunately for the logs in the shell, it only shows the first argument sent to console.error(), so it won't show the stack trace there, only the prefix. Like this: [88176:88176:1017/131257.231940:INFO:CONSOLE(75)] "[unhandled-error]: Unknown error.", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js (75) [88176:88176:1017/131257.277945:INFO:CONSOLE(75)] "[unhandled-error]: Error retrieving Web Store access token.", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js (75)
,
Oct 17
> Unfortunately for the logs in the shell, it only shows the first argument sent to console.error(), so it won't show the stack trace there, only the prefix. :( an array.join() might help capture all, rather than the first.
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be1e48891cac2e14e1f8cfd48cb69a396dc57f55 commit be1e48891cac2e14e1f8cfd48cb69a396dc57f55 Author: Luciano Pacheco <lucmult@chromium.org> Date: Wed Oct 17 06:05:30 2018 Update error_counter.js to ES6 styles Update comments for functions. Update closure markup and binding arguments for |wrap| function because this is used in Video Player app. ES6 styles: - Convert var to const. - Convert most anonymous functions to arrow function. Keeping |wrappedCallback| because it shows a bit better in the stack trace. - Convert implicit arguments to ...args, because arguments doesn't work in arrow functions. - Add 'use strict' with anonymous namespace because grit in-lines all JS files together. Bug: 893430 Change-Id: Icaa6271e91b446efe14abc06fbbb44589fac20b7 Cq-Include-Trybots: luci.chromium.try:closure_compilation Reviewed-on: https://chromium-review.googlesource.com/c/1282683 Reviewed-by: Trent Apted <tapted@chromium.org> Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#600296} [modify] https://crrev.com/be1e48891cac2e14e1f8cfd48cb69a396dc57f55/ui/file_manager/base/js/error_counter.js
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c5242283d9b5f3c12b73db3d31f5a21d367b0571 commit c5242283d9b5f3c12b73db3d31f5a21d367b0571 Author: Luciano Pacheco <lucmult@chromium.org> Date: Wed Oct 17 06:55:21 2018 Add error stack for unhandled errors and asserts Add prefix [unhandled-error] for logs via console.error() to make them easier to identify among other console.log() messages. Add error stack for console.error() and console.assert() to make easier to identify which code actually issued the call. Bug: 893430 Change-Id: I3ab98f6d40c76ee1fb8f5eebce2ad4911ac4e7e0 Reviewed-on: https://chromium-review.googlesource.com/c/1282686 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#600305} [modify] https://crrev.com/c5242283d9b5f3c12b73db3d31f5a21d367b0571/ui/file_manager/base/js/error_counter.js
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6 commit c4d25f5b67f67cf274db8a2fdc9b03342f113ac6 Author: Luciano Pacheco <lucmult@chromium.org> Date: Wed Oct 17 07:16:33 2018 Unify JSErrorCounter for Files, Audio, Video and Gallery apps. Bug: 893430 Change-Id: I4cb8149eaec9c9ab22700905dae804ac893fdb6c Reviewed-on: https://chromium-review.googlesource.com/c/1282517 Reviewed-by: Noel Gordon <noel@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#600308} [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/audio_player/js/BUILD.gn [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/audio_player/js/audio_player_scripts.js [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/audio_player/js/background_scripts.js [delete] https://crrev.com/68160a59d6a5aaf62bf6d8348358e9a9a1b618ac/ui/file_manager/audio_player/js/error_util.js [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/file_manager/background/js/background_common_scripts.js [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/file_manager/background/js/background_scripts.js [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/gallery/js/gallery_scripts.js [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/gallery/js/test_util.js [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/video_player/js/BUILD.gn [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/video_player/js/background_scripts.js [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/video_player/js/cast/BUILD.gn [delete] https://crrev.com/68160a59d6a5aaf62bf6d8348358e9a9a1b618ac/ui/file_manager/video_player/js/error_util.js [modify] https://crrev.com/c4d25f5b67f67cf274db8a2fdc9b03342f113ac6/ui/file_manager/video_player/js/video_player_scripts.js
,
Oct 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/506ce45de04c748b9c4481c2a1d43ffcd83d7692 commit 506ce45de04c748b9c4481c2a1d43ffcd83d7692 Author: Luciano Pacheco <lucmult@chromium.org> Date: Thu Oct 18 02:49:32 2018 Revert "Unify JSErrorCounter for Files, Audio, Video and Gallery apps." This reverts commit c4d25f5b67f67cf274db8a2fdc9b03342f113ac6. Reason for revert: This made some tests for Video and Audio players flaky crbug.com/896529 Original change's description: > Unify JSErrorCounter for Files, Audio, Video and Gallery apps. > > Bug: 893430 > Change-Id: I4cb8149eaec9c9ab22700905dae804ac893fdb6c > Reviewed-on: https://chromium-review.googlesource.com/c/1282517 > Reviewed-by: Noel Gordon <noel@chromium.org> > Reviewed-by: Trent Apted <tapted@chromium.org> > Commit-Queue: Luciano Pacheco <lucmult@chromium.org> > Cr-Commit-Position: refs/heads/master@{#600308} TBR=tapted@chromium.org,noel@chromium.org,lucmult@chromium.org Change-Id: I676ef1e26b98ff842158501ebf1221520db18bc9 Bug: 896529 Reviewed-on: https://chromium-review.googlesource.com/c/1287729 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#600642} [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/audio_player/js/BUILD.gn [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/audio_player/js/audio_player_scripts.js [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/audio_player/js/background_scripts.js [add] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/audio_player/js/error_util.js [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/file_manager/background/js/background_common_scripts.js [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/file_manager/background/js/background_scripts.js [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/gallery/js/gallery_scripts.js [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/gallery/js/test_util.js [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/video_player/js/BUILD.gn [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/video_player/js/background_scripts.js [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/video_player/js/cast/BUILD.gn [add] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/video_player/js/error_util.js [modify] https://crrev.com/506ce45de04c748b9c4481c2a1d43ffcd83d7692/ui/file_manager/video_player/js/video_player_scripts.js |
||||
►
Sign in to add a comment |
||||
Comment 1 by lucmult@chromium.org
, Oct 9