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 Netkan error message when both ksp_version and ksp_version_min/_max are present #2480

Merged
merged 1 commit into from Aug 5, 2018

Conversation

@HebaruSan
Copy link
Member

commented Aug 4, 2018

Problem

KSP-CKAN/NetKAN#6654 got tripped up by this very confusing error message:

9493 [1] FATAL CKAN.NetKAN.Program (null) - Exception has been thrown by the target of an invocation.

The root cause was that the generated metadata had this:

  "ksp_version": "1.4.0",
  "ksp_version_min": "1.4.0",
  "ksp_version_max": "1.4.99",

These properties aren't supposed to be combined; either you have ksp_version, or you have _min and/or _max. We have a check for that with its own special exception here:

// Now see if we've got version with version min/max.
if (ksp_version != null && (ksp_version_max != null || ksp_version_min != null))
{
// KSP version mixed with min/max.
throw new InvalidModuleAttributesException("ksp_version mixed with ksp_version_(min|max)", this);
}

However, that info wasn't making it all the way to the output.

Changes

Netkan now uses Exception.GetBaseException() to strip away useless wrapper exceptions.

InvalidModuleAttributesException now passes its why parameter to its base class's constructor, which populates Exception.Message for Netkan to print out.

Now the error looks like this instead:

1344 [1] FATAL CKAN.NetKAN.Program (null) - ksp_version mixed with ksp_version_(min|max)
@politas

This comment has been minimized.

Copy link
Member

commented Aug 5, 2018

Cool! That confusing error message has lead to much scratching of heads!

@politas politas merged commit 6661602 into KSP-CKAN:master Aug 5, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
politas added a commit that referenced this pull request Aug 5, 2018
@HebaruSan HebaruSan deleted the HebaruSan:fix/netkan-version-error branch Aug 5, 2018
@linuxgurugamer

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2018

This comment:

The root cause was that the generated metadata had this:

"ksp_version": "1.4.0",
"ksp_version_min": "1.4.0",
"ksp_version_max": "1.4.99",
These properties aren't supposed to be combined; either you have ksp_version, or you have _min and/or _max. We have a check for that with its own special exception here:

is wrong. While duplicative, it is fine to have both. If min/max are there, the ksp_version is ignored.

@HebaruSan

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2018

That comment is correct. CKAN throws an exception in that case:

// Now see if we've got version with version min/max.
if (ksp_version != null && (ksp_version_max != null || ksp_version_min != null))
{
// KSP version mixed with min/max.
throw new InvalidModuleAttributesException("ksp_version mixed with ksp_version_(min|max)", this);
}

@HebaruSan

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2018

I think you're thinking of how KSP-AVC treats .version files, which is not the same as how CKAN treats its module metadata.

@linuxgurugamer

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2018

oops, you are correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.