Skip to content

[Feedback] Do not minify feedback issue body #17353

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

Merged
jiasli merged 4 commits into from
Apr 9, 2021
Merged

[Feedback] Do not minify feedback issue body #17353

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f11784d
not minify
jiasli Mar 15, 2021
94cdb44
Format issue_body only once
jiasli Apr 7, 2021
43a7f20
Merge branch 'dev' into feedback
jiasli Apr 7, 2021
7e91047
Remove comment
jiasli Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
257 changes: 30 additions & 227 deletions src/azure-cli/azure/cli/command_modules/feedback/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,21 @@
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

import os
import re
import datetime
import math
import os
import platform
import datetime

try:
from urllib.parse import urlencode # python 3
except ImportError:
from urllib import urlencode # python 2

from collections import namedtuple
from urllib.parse import urlencode

from azure.cli.core.azlogging import _UNKNOWN_COMMAND, _CMD_LOG_LINE_PREFIX
from azure.cli.core.commands.constants import SURVEY_PROMPT
from azure.cli.core.extension._resolve import resolve_project_url_from_index, NoExtensionCandidatesError
from azure.cli.core.util import get_az_version_string, open_page_in_browser, can_launch_browser, in_cloud_console
from knack.log import get_logger
from knack.prompting import prompt, NoTTYException
from knack.util import CLIError

from azure.cli.core.extension._resolve import resolve_project_url_from_index, NoExtensionCandidatesError
from azure.cli.core.util import get_az_version_string, open_page_in_browser, can_launch_browser, in_cloud_console
from azure.cli.core.azlogging import _UNKNOWN_COMMAND, _CMD_LOG_LINE_PREFIX
from azure.cli.core.commands.constants import SURVEY_PROMPT

_ONE_MIN_IN_SECS = 60

_ONE_HR_IN_SECS = 3600
Expand Down Expand Up @@ -57,18 +50,14 @@

_MSG_ISSUE = "Would you like to create an issue? Enter Y or N: "

_ISSUES_TEMPLATE_PREFIX = """

BEGIN TEMPLATE
===============
**If possible, a browser will be opened to {} to create an issue.**
**You can also run `az feedback --verbose` to emit the full issue draft to stderr.**
**Azure CLI repo: {}**
**Azure CLI Extensions repo: {}**
_OPEN_BROWSER_INSTRUCTION = """
* If possible, a browser will be opened to {} to create an issue.
* If the URL length exceeds GitHub or the browser's limitation and causes the content to be trimmed, you can run `az feedback --verbose` to emit the full issue draft to stderr.
* Azure CLI repo: {}
* Azure CLI Extensions repo: {}
"""

_ISSUES_TEMPLATE = """

### **This is autogenerated. Please review and update as needed.**

## Describe the bug
Expand All @@ -77,7 +66,9 @@
`{command_name}`

**Errors:**
```
{errors_string}
```

## To Reproduce:
Steps to reproduce the behavior. Note that argument values have been redacted, as they may contain sensitive information.
Expand Down Expand Up @@ -348,160 +339,6 @@ def _get_info_from_log_line(line, p_id):
return CommandLogFile._LogRecordType(*parts)


class ErrorMinifier:

_FILE_RE = re.compile(r'File "(.*)"')
_CONTINUATION_STR = "...\n"

def __init__(self, errors_list):
self._errors_list = errors_list
self._capacity = None # to how many symbols should minify
self._minified_error = "\n".join(self._errors_list)

def set_capacity(self, capacity):
logger.debug("Capacity for error string: %s", capacity)

self._capacity = int(capacity)
self._minified_error = self._get_minified_errors()

def _get_minified_errors(self): # pylint: disable=too-many-return-statements
errors_list = self._errors_list
errors_string = "\n".join(errors_list)
if self._capacity is None:
return errors_string

if not errors_list:
return ""

# if within capacity return string
if len(errors_string) <= self._capacity:
return errors_string

# shorten file names and try again
for i, error in enumerate(errors_list):
errors_list[i] = self._minify_by_shortening_file_names(error, levels=5)
errors_string = "\n".join(errors_list)
if len(errors_string) <= self._capacity:
return errors_string

# shorten file names and try again
for i, error in enumerate(errors_list):
errors_list[i] = self._minify_by_shortening_file_names(error, levels=4)
errors_string = "\n".join(errors_list)
if len(errors_string) <= self._capacity:
return errors_string

# return first exception if multiple exceptions occurs
for i, error in enumerate(errors_list):
errors_list[i] = self._minify_by_removing_nested_exceptions(error)
errors_string = "\n".join(errors_list)
if len(errors_string) <= self._capacity:
return errors_string

# remove some lines
errors_string = self._minify_by_removing_lines(errors_string, desired_length=self._capacity)
if len(errors_string) <= self._capacity:
return errors_string

# last resort: strip off the suffix
return self._minify_by_removing_suffix(errors_string, desired_length=self._capacity)

@staticmethod
def _minify_by_shortening_file_names(error_string, levels=5):
new_lines = []
for line in error_string.splitlines():
# if original exception
if line.strip().startswith("File") and ", line" in line:
parts = line.split(",")
match = ErrorMinifier._FILE_RE.search(parts[0])
if match:
parts[0] = ErrorMinifier._shorten_file_name(match.group(1), levels)
parts[1] = parts[1].replace("line", "ln")
line = ",".join(parts)
# if cleaned exceptions
elif ".py" in line and ", ln" in line:
parts = line.split(",")
parts[0] = ErrorMinifier._shorten_file_name(parts[0], levels)
line = ",".join(parts)
# remove this line
elif "here is the traceback" in line.lower():
continue

new_lines.append(line)

return "\n".join(new_lines)

@staticmethod
def _shorten_file_name(file_name, levels=5):
if levels > 0:
new_name = os.path.basename(file_name)
file_name = os.path.dirname(file_name)
for _ in range(levels - 1):
new_name = os.path.join(os.path.basename(file_name), new_name)
file_name = os.path.dirname(file_name)
return new_name
return file_name

@staticmethod
def _minify_by_removing_nested_exceptions(error_string):
lines = error_string.splitlines()

idx = len(lines) - 1
for i, line in enumerate(lines):
if "During handling of the above exception" in line:
idx = i
break

# if unchanged return error_string
if idx == len(lines) - 1:
return error_string

lines = lines[:idx] + [ErrorMinifier._CONTINUATION_STR] + lines[-3:]
return "\n".join(lines)

@staticmethod
def _minify_by_removing_lines(error_string, desired_length):
"""
Repeatedly remove the lines from the middle, as a last resort remove even the
first line but keep the last one, in the effort to strip down the error string
to the desired length.
"""
if len(error_string) <= desired_length:
return error_string

symbols_to_remove = len(error_string) - desired_length + len(ErrorMinifier._CONTINUATION_STR)
lines = error_string.splitlines(keepends=True)

if len(lines) <= 1 or symbols_to_remove <= 0:
# nothing to remove
return error_string

mid = 0
while len(lines) > 1 and symbols_to_remove > 0:
# remove the middle line, when even prefer to remove the one closer to the start
mid = (len(lines) - 1) // 2
symbols_to_remove -= len(lines.pop(mid))

lines.insert(mid, ErrorMinifier._CONTINUATION_STR)
return "".join(lines)

@staticmethod
def _minify_by_removing_suffix(error_string, desired_length):
"""
Strip off the suffix of the error string, force it to be <= desired length.
"""
if len(error_string) <= desired_length:
return error_string

continuation = ErrorMinifier._CONTINUATION_STR.strip()[:desired_length]
return error_string[:desired_length - len(continuation)] + continuation

def __str__(self):
if self._minified_error:
return "```\n{}\n```".format(self._minified_error.strip())
return ""


def _build_issue_info_tup(command_log_file=None):
format_dict = {"command_name": "", "errors_string": "",
"executed_command": ""}
Expand All @@ -525,7 +362,7 @@ def _build_issue_info_tup(command_log_file=None):
is_ext = True
ext_name = extension_name

format_dict["errors_string"] = ErrorMinifier(errors_list)
format_dict["errors_string"] = ''.join(errors_list)
format_dict["executed_command"] = "az " + executed_command if executed_command else executed_command
format_dict["command_name"] += extension_info

Expand All @@ -540,36 +377,13 @@ def _build_issue_info_tup(command_log_file=None):

pretty_url_name = _get_extension_repo_url(ext_name) if is_ext else _CLI_ISSUES_URL

# get issue body without minification
original_issue_body = _ISSUES_TEMPLATE.format(**format_dict)

# First try
capacity = _MAX_URL_LENGTH # some browsers support a max of roughly 2000 characters
res = _get_minified_issue_url(command_log_file, format_dict.copy(), is_ext, ext_name, capacity)
formatted_issues_url, minified_issue_body = res
capacity = capacity - (len(formatted_issues_url) - _MAX_URL_LENGTH)

# while formatted issue to long, minify to new capacity
tries = 0
while len(formatted_issues_url) > _MAX_URL_LENGTH and tries < 25:
# reduce capacity by difference if formatted_issues_url is too long because of url escaping
res = _get_minified_issue_url(command_log_file, format_dict.copy(), is_ext, ext_name, capacity)
formatted_issues_url, minified_issue_body = res
capacity = capacity - (len(formatted_issues_url) - _MAX_URL_LENGTH)
tries += 1

# if something went wrong with minification (i.e. another part of the issue is unexpectedly too long)
# then truncate the whole issue body and warn the user.
if len(formatted_issues_url) > _MAX_URL_LENGTH:
formatted_issues_url = formatted_issues_url[:_MAX_URL_LENGTH]
logger.warning("Failed to properly minify issue url. "
"Please use 'az feedback --verbose' to get the full issue output.")

logger.debug("Total minified issue length is %s", len(minified_issue_body))
issue_body = _ISSUES_TEMPLATE.format(**format_dict)
formatted_issues_url = _get_issue_url(command_log_file, issue_body, is_ext, ext_name)

logger.debug("Total formatted url length is %s", len(formatted_issues_url))

return _ISSUES_TEMPLATE_PREFIX.format(pretty_url_name, _CLI_ISSUES_URL, _EXTENSIONS_ISSUES_URL), \
formatted_issues_url, original_issue_body
return _OPEN_BROWSER_INSTRUCTION.format(pretty_url_name, _CLI_ISSUES_URL, _EXTENSIONS_ISSUES_URL), \
formatted_issues_url, issue_body


def _get_extension_repo_url(ext_name, raw=False):
Expand All @@ -596,32 +410,20 @@ def _is_valid_github_project_url(url):
return False


def _get_minified_issue_url(command_log_file, format_dict, is_ext, ext_name, capacity):
# get issue body without errors
minified_errors = format_dict["errors_string"]
format_dict["errors_string"] = ""
no_errors_issue_body = _ISSUES_TEMPLATE.format(**format_dict)

# get minified issue body
format_dict["errors_string"] = minified_errors
if hasattr(minified_errors, "set_capacity"):
logger.debug("Length of issue body before errors added: %s", len(no_errors_issue_body))
minified_errors.set_capacity(
capacity - len(no_errors_issue_body)) # factor in length of url and expansion of url escaped characters
minified_issue_body = _ISSUES_TEMPLATE.format(**format_dict)

def _get_issue_url(command_log_file, issue_body, is_ext, ext_name):
# prefix formatted url with 'https://' if necessary and supply empty body to remove any existing issue template
# aka.ms doesn't work well for long urls / query params
formatted_issues_url = _get_extension_repo_url(ext_name, raw=True) if is_ext else _RAW_CLI_ISSUES_URL
if not formatted_issues_url.startswith("http"):
formatted_issues_url = "https://" + formatted_issues_url
query_dict = {'body': minified_issue_body}
# https://docs.github.com/en/github/managing-your-work-on-github/about-automation-for-issues-and-pull-requests-with-query-parameters
query_dict = {'body': issue_body}
if command_log_file and command_log_file.failed():
query_dict['template'] = 'Bug_report.md'
new_placeholder = urlencode(query_dict)
formatted_issues_url = "{}?{}".format(formatted_issues_url, new_placeholder)

return formatted_issues_url, minified_issue_body
return formatted_issues_url


def _get_az_version_summary():
Expand Down Expand Up @@ -748,19 +550,20 @@ def _prompt_issue(recent_command_list):

if ans in ["y", "n"]:
if ans == "y":
prefix, url, original_issue = _build_issue_info_tup()
browser_instruction, url, issue_body = _build_issue_info_tup()
else:
return False
else:
if ans in ["q", "quit"]:
return False
if ans == 0:
prefix, url, original_issue = _build_issue_info_tup()
browser_instruction, url, issue_body = _build_issue_info_tup()
else:
prefix, url, original_issue = _build_issue_info_tup(recent_command_list[ans])
print(prefix)
browser_instruction, url, issue_body = _build_issue_info_tup(recent_command_list[ans])

logger.info(issue_body)
print(browser_instruction)

logger.info(original_issue)
# if we are not in cloud shell and can launch a browser, launch it with the issue draft
if can_launch_browser() and not in_cloud_console():
open_page_in_browser(url)
Expand Down