Skip to content

(Willing to PR) _debugReportException should be _reportException indeed, otherwise seems misleading to the readers because it is not debug-only #111874

New issue
New issue
Closed
#114117
Closed
(Willing to PR) _debugReportException should be _reportException indeed, otherwise seems misleading to the readers because it is not debug-only#111874
#114117
Labels
P3Issues that are less important to the Flutter projectc: proposalA detailed proposal for a change to Flutterframeworkflutter/packages/flutter repository. See also f: labels.r: fixedIssue is closed as already fixed in a newer version
@fzyzcjy

Description

@fzyzcjy
fzyzcjy
opened on Sep 19, 2022

Steps to Reproduce

Hi thanks for the wonderful framework! When reading the source code, I see:

flutter/packages/flutter/lib/src/rendering/object.dart

Lines 1531 to 1550 in 05c7052

void _debugReportException(String method, Object exception, StackTrace stack) {
FlutterError.reportError(FlutterErrorDetails(
exception: exception,
stack: stack,
library: 'rendering library',
context: ErrorDescription('during $method()'),
informationCollector: () => <DiagnosticsNode>[
// debugCreator should always be null outside of debugMode, but we want
// the tree shaker to notice this.
if (kDebugMode && debugCreator != null)
DiagnosticsDebugCreator(debugCreator!),
describeForError('The following RenderObject was being processed when the exception was fired'),
// TODO(jacobr): this error message has a code smell. Consider whether
// displaying the truncated children is really useful for command line
// users. Inspector users can see the full tree by clicking on the
// render object so this may not be that useful.
describeForError('RenderObject', style: DiagnosticsTreeStyle.truncateChildren),
],
));
}

and various places which calls it. The problem is, if I understand correctly, "debugSomething" always means "this thing is only valid when in debug mode and will disappear in release mode", such as debugDoingThisResize, debugDoingThisLayout, etc. For completeness, I briefly glances through other symbols/functions with name debug and _debug.*( in rendering/objects.dart, and find that they are used inside assert(...) or used with kDebugMode.

However, this method name indeed reports error unconditionally, i.e. report both in debug and release. That is pretty reasonable, because we surely want error reports in release builds. The problem is the naming - the name seems confusing to the readers IMHO.

Thus, I propose to simply rename _debugReportException to _reportException.

I am willing to make a PR. However, given that this is just a code renaming, I wonder whether I should create a PR?

  1. Execute flutter run on the code sample
  2. ...
  3. ...

Expected results:

Actual results:

Code sample 





  Logs












Metadata
Metadata
Assignees
No one assigned
    Labels
    P3Issues that are less important to the Flutter projectc: proposalA detailed proposal for a change to Flutterframeworkflutter/packages/flutter repository. See also f: labels.r: fixedIssue is closed as already fixed in a newer version
    Type
    No type
    Projects
    No projects
    Milestone
    No milestone
    Relationships
    None yet
    Development
    No branches or pull requests
    Issue actions