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

Add content logging to help debug a Story post crash #14191

Merged
merged 4 commits into from
Mar 4, 2021

Conversation

renanferrari
Copy link
Member

This PR is a follow up to #14185 and helps to investigate #13697.

It basically adds a SiteModel parameter to the findAllStoryBlocksInPostAndPerformOnEachMediaFilesJson method which we then use to check if we should also log the Post's content in addition to reporting any caught StringIndexOutOfBoundsException. The logic used to determine that is based on this one, which makes sure we only log posts that are not private or password-protected, on WPCom sites that are also not private.

To test:

Again, there's not much to test here since we can't reproduce the issue, but you can smoke test the Story feature and make sure nothing looks off.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Mar 4, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Mar 4, 2021

You can test the changes on this Pull Request by downloading the APK here.

@mzorz mzorz self-assigned this Mar 4, 2021
Copy link
Contributor

@mzorz mzorz left a comment

Choose a reason for hiding this comment

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

Thank you for these changes @renanferrari 💯

I was able to test the try/catch path by adding an artificial throw StringIndexOutOfBoundsException("TEST") line within the try{} block, and confirmed the logcat showed what I expected:

2021-03-04 18:17:21.062 15273-15273/org.wordpress.android E/WordPress-EDITOR: Error while parsing Story blocks: TEST
2021-03-04 18:17:24.027 15273-15333/org.wordpress.android D/Volley: [534] BasicNetwork.logSlowRequests: HTTP response for request=<[ ] https://public-api.wordpress.com/wpcom/v2/sites/176139122/rewind/capabilities/?_locale=en_US 0x7b2dc254 NORMAL 39> [lifetime=20476], [size=19], [rc=200], [retryCount=0]
2021-03-04 18:17:29.517 15273-15273/org.wordpress.android E/WordPress-EDITOR: HTML content of the post before the crash: <!-- wp:jetpack/story {"mediaFiles":[{"alt":"asdfafdafa","caption":"","id":"1","link":"","mime":"image/jpeg","type":"image","url":""}]} -->
    <div class="wp-story wp-block-jetpack-story"></div>
    <!-- /wp:jetpack/story -->

Also confirmed the test case runs correctly
Finally, I gave a pass on the code which looks right and to the point 👍

LGTM :shipit:

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

Successfully merging this pull request may close these issues.

3 participants