GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Log err.message instead of <unavailable> err object by Eselce · Pull Request #2939 · greasemonkey/greasemonkey

Conversation

@Eselce

@Eselce Eselce commented Apr 9, 2018

Copy link
Copy Markdown
Contributor

Most of the log messages printing a caught error object err (or chrome.runtime.lastError) were converted to <unavailable>, which is not very helpful. But then err.message provides the information needed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Eselce added 2 commits April 9, 2018 16:14
@Eselce
Log err.message instead of <unavailable> err object
48caa8c
@Eselce
Unnecessary blank line
f6a5336
@Sxderp

Sxderp commented Apr 9, 2018

Copy link
Copy Markdown
Contributor

As an aside, I think it depends on which console you're looking at. There're three consoles; extension debugging, browser, and page. One of them is bound to actually provide access to the object.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Eselce

Eselce commented Apr 9, 2018

Copy link
Copy Markdown
Contributor Author

@Sxderp Yes, that's correct. I played around with exceptions, errors, etc., and there are quite a few occations, when you get a full output on the (tool) Ctrl-Shift-I console (provided, that the filter is set approriately), even with a stack trace. Many useful logs go to the browser (web) Ctrl-Shift-J console, where access is limited. Debugging can provide the object, but that has nothing to do with these log messages.

Maybe there is a case, where we would like to see the whole object (and it has a nice output), but for me, it looks as if I often miss a helpful error message, if an error occurs. Tell me, if there are really many cases (apart from debugging with break points and value evaluation), where you need the whole error object (in the case of err = chrome.runtime.lastError, documentation tells me, that only err.message is guaranteed)...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@arantius

arantius commented Apr 9, 2018

Copy link
Copy Markdown
Collaborator

Please help me understand/test: Clear reproduction case where the effect before this change is undesirable?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Eselce

Eselce commented Apr 9, 2018

Copy link
Copy Markdown
Contributor Author

Please help me understand/test: Clear reproduction case where the effect before this change is undesirable?

I don't know, if I understood your question right, but here is my "problem" with the current output:

Now and then, while testing, an unexpected error occurs. Reason: wrong data, a programming error, sth. weird.
The result may be, that scripts are (silently) not executed, not loaded, not installed.
Then I check the console output(s) to look for error output (which is mostly the case, either on the web or on the tool console). I want a sensible message text for help, but what I get, is something like Error: <unavailable>. Not very helpful. Maybe I could get more information using the debugger, but for most of these errors, I just need a hint, not the error type or the stack trace. It's unnecessary uninformative...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sxderp
Sxderp reviewed Apr 9, 2018
View reviewed changes
Comment thread src/bg/user-script-registry.js Outdated
}).catch(err => {
console.error('Error in installFromDownloader()', err);
// Rethrow so caller can also deal with it
console.error('Error in installFromDownloader():', err.message);

@Sxderp Sxderp Apr 9, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could be wrong about this, but I don't think indexedDB errors are guaranteed to have a .message property. Might want to force a ConstraintError (as an example) and see if it has a message. If it doesn't then this may need to be changed to err.message || err.name.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@Eselce Eselce Apr 9, 2018

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a good idea for every such error message (except for chrome.runtime.lastError).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@Eselce
Use err.name as backup for err.message
8845ccb
@arantius

arantius commented Jul 27, 2018

Copy link
Copy Markdown
Collaborator

Sorry I forgot about this for a while, now it won't merge cleanly.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Eselce
Merge branch 'master' into log-err-message-not-unavailable
6829e4d
@arantius arantius added this to the 4.8 milestone Aug 22, 2018
arantius added a commit to arantius/greasemonkey that referenced this pull request May 8, 2019
@arantius
Merge branch 'log-err-message-not-unavailable'
9633490
Fixes greasemonkey#2939
@arantius
arantius merged commit 6829e4d into greasemonkey:master May 8, 2019
@Cerberus-tm

Cerberus-tm commented May 11, 2019

Copy link
Copy Markdown
Contributor

@arantius Thank you for all these recent fixes. Very nice.

Eselce reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@Sxderp Sxderp Sxderp left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.8

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@Eselce @Sxderp @arantius @Cerberus-tm
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.