Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@iskakaushik
Copy link
Contributor

This is to reduce log spam.

Fixes: flutter/flutter#55966

@iskakaushik iskakaushik requested a review from gw280 April 30, 2020 21:20
@auto-assign auto-assign bot requested a review from chinmaygarde April 30, 2020 21:20
FML_LOG(ERROR)
<< "Platform view received message on channel '" << message->channel()
<< "' with no registered handler. And empty response will be "
"generated. Please implement the native message handler.";
Copy link
Contributor

Choose a reason for hiding this comment

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

Just mention that this message will only be logged once.

@iskakaushik iskakaushik added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 30, 2020
<< "Platform view received message on channel '" << message->channel()
<< "' with no registered handler. And empty response will be "
"generated. Please implement the native message handler.";
const bool already_errored = unregistered_channels_.count(channel);
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this additional check? if we're not in unregistered_channels_ then we've never logged before. This currently will log if we get a message on e.g. channelA, then channelB, channelC, etc won't log (as count() will be > 0).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is count(channel), as in a count to check if the channel is in the set.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh I misread. sorry!

// These are the channels that aren't registered and have been notified as
// such. Notifying via logs multiple times results in log-spam. See:
// https://github.com/flutter/flutter/issues/55966
std::set<std::string /* channel */> unregistered_channels_;
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have multiple PlatformViews over the lifetime of a process? do we want this to be a static?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's only one platform view per engine so I think that should be fine. Given that what we are seeing now, it this message being printed at error levels every time we receive a platform message on the same channel, I changed it to be INFO and made it so that we log a channel once per engine. So I think this should reduce the problem by a lot. If we notice that we are still seeing spammy behavior making this a static is something we could consider.

@iskakaushik iskakaushik merged commit 6530b57 into flutter:master Apr 30, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 30, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ERROR:flutter/shell/platform/fuchsia/flutter/platform_view.cc

4 participants