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

more detailed error message for generator #1013

Merged

Conversation

SabotageAndi
Copy link
Contributor

No description provided.


return ex.Message + " -> " + GetMessage(ex.InnerException);
return ex + " -> " + GetMessage(ex.InnerException);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think ex will contain the inner exception as well, so it does not make sense to call the GetMessage for the inner exception

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. Did some testing with it.

@gasparnagy
Copy link
Contributor

@SabotageAndi is this a temporary change, or do you want to include it as a final solution?

@SabotageAndi
Copy link
Contributor Author

I was more thinking of a permanent change.
The errors that are printed in the code- generation into the feature.cs files are not helping at all.

I am stuck with SpecFlowOSS/SpecFlow.VisualStudio#26 at an FileLoadException and I only get the message. No idea what is failing.

@SabotageAndi
Copy link
Contributor Author

The output would be now this:

image

@gasparnagy
Copy link
Contributor

@SabotageAndi I see. I think it would still make sense to separate the readable error message from the full stacktrace. My suggestion would be to keep the readable error message (the way it was working so far) in the first line and only add the full stack trace from the second line on...
On SpecFlowOSS/SpecFlow.VisualStudio#26 i have big fears if this will really work in all cases. There might be pretty complex project setup (that we are not even aware of) that might require the AppDomain in the right folder. Can't we move this all to process-based communication if we make any change?

@gasparnagy
Copy link
Contributor

@SabotageAndi i see your screenshot now. looks good enough.

@SabotageAndi
Copy link
Contributor Author

@gasparnagy Yes, out of proc would solve a lot of problems, but it isn't done in 5 mins.

@gasparnagy
Copy link
Contributor

@SabotageAndi sure/ack. but as far as I see SpecFlowOSS/SpecFlow.VisualStudio#26, that was also not a 5 min stuff...

Andreas Willich added 3 commits January 30, 2018 15:01
@SabotageAndi
Copy link
Contributor Author

@gasparnagy I changed the error message to:

first line exception message
empty Line
whole exception as a string

Ok with that? Than I will merge this in.

@gasparnagy gasparnagy merged commit d2db36b into SpecFlowOSS:master Jan 30, 2018
@SabotageAndi SabotageAndi deleted the MoreDetailedGenerationErrorMessages branch January 30, 2018 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants