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

fix: preview show error stack trace #6281

Merged

Conversation

devnamrits
Copy link
Contributor

@devnamrits devnamrits commented Mar 8, 2022

Closes #5907 #4070

Summary:
It will elaborately describe the error by showing the stack trace of the error.

As of now, only the error message is being shown in the UI which makes it difficult to debug things in the app.

Test plan
Screenshot 2022-03-08 at 9 50 07 PM

Checklist

Please add a x inside each checkbox:

  • I have read the contribution guidelines.
  • Code is formatted via running yarn format.
  • Tests are passing via running yarn test.
  • The status checks are successful (continuous integration). Those can be seen below.

@devnamrits
Copy link
Contributor Author

@erezrokah
I have raised draft PR. What's next?? Will it be merged??
Or, meanwhile, I've to wait?? If I've to wait then can I jump to other issues??

Please explain the further procedure. Sorry for being so silly.

@erezrokah erezrokah added the type: bug code to address defects in shipped code label Mar 8, 2022
@erezrokah erezrokah changed the title Fix/preview show error stack trace fix: preview show error stack trace Mar 8, 2022
@erezrokah
Copy link
Contributor

Thanks @devnamrits, give me some time to test this.

You can look at other issues if you'd like

@devnamrits
Copy link
Contributor Author

@erezrokah
Thank you for your kind response. Meanwhile, I'm looking into other issues.

@erezrokah
Copy link
Contributor

@devnamrits, looks like some CI tests are failing, can you look into it?
You can those via yarn test. See https://github.com/netlify/netlify-cms/blob/964d6975dff599dd9afa55deae9c54a6630c0d96/CONTRIBUTING.md#test

@devnamrits
Copy link
Contributor Author

@erezrokah Looking into this.

@devnamrits
Copy link
Contributor Author

@erezrokah Created a new state variable named as errorTitle because while creating the issue URL we're using just errorMessage rather than using the whole error stack. Also since we're generating snapshots for tests we run. In the case of ErrorBoundary.js, we're generating a snapshot of the error UI which will contain the error trace. So the snapshots will contain those error traces as well which will vary from user to user due to different local directory configurations. Hence, after updating test snapshots, all tests are passing.
Screenshot 2022-03-16 at 7 55 01 PM

@erezrokah erezrokah force-pushed the fix/preview-show-error-stack-trace branch from 46b26f4 to 51d62f6 Compare April 13, 2022 12:44
@erezrokah erezrokah marked this pull request as ready for review April 13, 2022 12:46
@erezrokah erezrokah requested a review from a team April 13, 2022 12:46
@erezrokah erezrokah force-pushed the fix/preview-show-error-stack-trace branch from 9633c67 to 1759be9 Compare April 13, 2022 12:59
@erezrokah
Copy link
Contributor

Thanks for this @devnamrits, I removed the snapshot validation as it does more harm than good in this case.
I also added some logic to cleanup the stack trace, see
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preview show error when change name of Cover Image field
2 participants