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.

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

Report dev error at viewer channel timeout #25931

Merged
merged 2 commits into from
Dec 17, 2019

Conversation

zhouyx
Copy link
Contributor

@zhouyx zhouyx commented Dec 7, 2019

Fixes #25923

Another approach I've been thinking of is to introduce an opt_errorType param to the #reportError() method. But that doesn't apply to the uncaught error that we ret hrow.

The code looks like

    return Services.timerFor(this.win)
      .timeoutPromise(20000, messagingPromise, 'initMessagingChannel')
      .catch(reason => {
        const error = getChannelError(
          /** @type {!Error|string|undefined} */ (reason)
        );
        reportError(error);
        throw error;
      });

Do you know why we want to re-throw the error after calling reportError() (From my test, either one should get the error sent to endpoint)

}
return new Error('No messaging channel: ' + opt_reason);
// Force convert user error to dev error
channelError.message = channelError.message.replace(USER_ERROR_SENTINEL, '');
Copy link
Contributor

Choose a reason for hiding this comment

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

This is, imho, too nuanced to spread from the log system. We already export isUserError there. Can we also expose something like stripUserError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I agree a helper method would be helpful if there are future use cases. Are you thinking of something like

error.message = stripUserError(error.message)

Or something like

error = convertToDevError(error)

@zhouyx
Copy link
Contributor Author

zhouyx commented Dec 17, 2019

Friendly ping : )

@zhouyx zhouyx merged commit 040f405 into ampproject:master Dec 17, 2019
@zhouyx zhouyx deleted the timeout-error branch December 17, 2019 22:09
micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this pull request Dec 27, 2019
* report dev error when viewer channel timeout

* use helper method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retag initMessagingChannel timeout error as a dev error
3 participants