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.

Sign up for GitHub

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

Jump to bottom

Provide easy way to restart NVDA with Debug level logging enabled #6689

Closed
feerrenrut opened this issue Dec 30, 2016 · 9 comments
Closed

Provide easy way to restart NVDA with Debug level logging enabled #6689

feerrenrut opened this issue Dec 30, 2016 · 9 comments
Assignees
@feerrenrut
Labels
p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority quick fix
Milestone
2017.1

Comments

@feerrenrut
Copy link
Contributor

feerrenrut commented Dec 30, 2016 •
edited

While implementing the profile/config upgrade work (PR #6558) it became clear that during the early stages of NVDA loading the logging level has not yet been set from the config file. In order to get debug level logging for these early stages you have to run NVDA with a command line argument to set the log level to debug (E.G. pythonw nvda.pyw -l 10). This is hard to remember, and would be hard to ask end users to do.

Firstly, I propose we add a more memorable option, for instance pythonw nvda.pyw --debug. In extension to this, making it easier for an end user to temporarily enable debug level logging would also lower the barrier to getting debug logs attached to github issues. This could be done by adding a "restart with debug logging" option to the tools menu of NVDA. Selecting this would restart NVDA passing the previously mentioned --debug argument.

The benefit of this is for users who generally do not want debug level logging, and the debug level logging is enabled earlier in the NVDA startup.
@feerrenrut feerrenrut added p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority quick fix labels Dec 30, 2016
@josephsl
Copy link
Collaborator

Hi,

I see potential use for this option.

As for where this new option should go, I think creating a checkbox in Exit NVDA dialog (NVDA+Q) would be nice, as users can be told to check this box by developers (the checkbox itself will be set to False, and if set to True, debug flag will be set in the next NVDA session) and could be found easily by users. Another option is to add an item in exit options list named "Restart with diagnostics enabled" that'll do what this issue is suggesting - I think this should be only available in development snapshots only.

Thanks.

@feerrenrut
Copy link
Contributor Author

Personally I think that wanting to restart with debug logging (for most users) is an edge case and thus should not be presented at all during a standard run of NVDA. I propose putting an action in the tools or help menu. This should be available in all builds of the application (including releases). This way its simple to ask anyone to restart with debug logging. Most users should not need it, and I'm not proposing this as a convenience for developers.

@josephsl
Copy link
Collaborator

Hi,

After thinking about it, I think a toggle should be added in Tools menu (similar to Speech Viewer), or an entry in that menu that'll ask users if they'd like to let the next NVDA session be a debugging one (or restart right then).

Thanks.

@feerrenrut
Copy link
Contributor Author

feerrenrut commented Dec 30, 2016 •
edited

For the sake of simplicity I would implement this so that the restart was immediate rather than trying to make it a deferrable action. Its important the next startup uses the -l 10 option (or equivalent) rather than altering the config file. Was your thinking around having a toggle option so that the user can check what state they are currently in?

@josephsl
Copy link
Collaborator

Hi,

My thinking is that it could be a toggle in that users can toggle it on if told to do so, with the previous logging behavior restored when NVDA restarts. In a way, users can do that right now by changing logging level option in General Settings. Restarts come in handy if troubleshooting startup problems such as an add-on failing to load, a synthesizer problem and so on.

I think we should gather more thoughts before proceeding so we can identify useful scenarios besides needing to start NVDA in debug mode. Thanks.

@feerrenrut
Copy link
Contributor Author

The main case I'm trying to cover with this issue is being able to provide a way for end users to get debug level logging during the initialisation of nvda. This issue is particularly in response to the addition of a profile/config schema upgrade step.

  • The upgrade step is a likely source of errors.
  • The upgrade step is hard for end users to enable debug logging for because it happens before the log level is read from the config.

The design I have proposed also brings the benefit of an easier way to temporarily get debug level logging, this is definitely a secondary concern and merely a "nice to have". By "temporarily" I mean that the debug logging is not automatically enabled after the next restart (via the standard exit dialog).

  • For users who are enabling / disabling debug logging, it would be able to be done in half as many steps.
  • Users would no longer having to remember to disable debug logging brings information security advantages too.

I think its great if we can come up with more use cases, however unless this design precludes some other important use case I would suggest that they are considered separately.

@Brian1Gaff
Copy link

Brian1Gaff commented Dec 30, 2016 via email

@feerrenrut
Copy link
Contributor Author

@Brian1Gaff No I'm not. I don't propose we remove the ability to set the log level on a more permanent basis (via the general settings dialog), this functionality is useful and there are many good reasons for users to wish to run NVDA with increased logging. Also, there has been no change to the point at which the logging level is set from the config.

By setting debug level logging as the default there are a few disadvantages:

  • We no longer have control over how much verbosity is put into the log. Log messages that may only be helpful during debugging would be present always.
  • We would like to have the option to be able to log the full .ini file before the schema upgrade process happens. Since addons store their config in the same file as the NVDA config we have no control over this information,. It may contain passwords etc. Uploading a log taken at an info level should never cause a security concern to the user.
  • More thought has to put into what information is leaked via debug level logging. Currently we only need consider what information is allowed to be leaked based on logging level. If the log level was debug by default, we would also need to consider if the code could be run before the log level is set from the .ini file.

I should point out that the solution I suggest would not be a costly change to implement, and will not take away any existing functionality.

@Brian1Gaff
Copy link

Brian1Gaff commented Dec 31, 2016 via email

feerrenrut added a commit that referenced this issue Jan 11, 2017
@feerrenrut
Add restart with debug level logging
6af0932 
See issue #6689
This provides a convenience to users who wish to enable debug level
logging temporarily (one run of nvda).
A start up option is added to make it easier to start nvda with debug
level logging. Enabling debug logging in this way sets the log level
earlier than setting it via the configuration file. In particular, this
allows the debug messages that occur during the loading of the
configuration file to be present in the log file.
@feerrenrut feerrenrut self-assigned this Jan 11, 2017
@feerrenrut feerrenrut mentioned this issue Jan 12, 2017
Merged
feerrenrut added a commit that referenced this issue Jan 19, 2017
@feerrenrut
incubates #6721
64f7053 
Re issue #6689
Merge branch 'i6689-restartWithDebugLogging' into next
@nvaccessAuto nvaccessAuto added this to the 2017.1 milestone Jan 31, 2017
feerrenrut added a commit that referenced this issue Jan 31, 2017
@feerrenrut
Update changes file for PR #6721
a61bf36 
- Added an option to the exit dialog to restart with debug level logging. (Issue #6689)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@feerrenrut feerrenrut

Labels
p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority quick fix
Projects
None yet
Milestone
2017.1
Development

No branches or pull requests
4 participants
@feerrenrut @josephsl @nvaccessAuto @Brian1Gaff