Skip to content

Add error message to report #163

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

Merged
merged 3 commits into from
May 28, 2016
Merged

Add error message to report #163

merged 3 commits into from
May 28, 2016

Conversation

rhart
Copy link

@rhart rhart commented May 19, 2016

Description

Adds a new field, message, to the report function.

Motivation and Context

Reporting the stackframes without the actual error message means you don't get the full context of the problem on the server you're reporting to.

How Has This Been Tested?

Existing test has been modified.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • node_modules/.bin/jscs -c .jscsrc stacktrace.js passes without errors
  • npm test passes without errors
  • I have read the contribution guidelines
  • I have updated the documentation accordingly
  • I have added tests to cover my changes

*
* @param {Array} stackframes
* @param {String} url
*/
report: function StackTrace$$report(stackframes, url) {
report: function StackTrace$$report(errorMsg, stackframes, url) {
Copy link
Member

Choose a reason for hiding this comment

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

Putting the error message first breaks the API, I'm afraid.

Copy link
Author

Choose a reason for hiding this comment

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

Are you suggesting to move it to last? Or is creating a completely separate method better?

Copy link
Member

Choose a reason for hiding this comment

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

Moving the argument would be sufficient. We may also only want to assign the message if it's populated.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@eriwen eriwen merged commit 6d2b6cb into stacktracejs:master May 28, 2016
eriwen added a commit that referenced this pull request May 28, 2016
@eriwen
Copy link
Member

eriwen commented May 28, 2016

Thank you @rhart - This has been published v1.2.0

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

Successfully merging this pull request may close these issues.

2 participants