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

Clean-up and debuggability #2399

Merged
merged 1 commit into from Apr 11, 2018
Merged

Conversation

HebaruSan
Copy link
Member

The past few weeks have seen a steady trickle of obscure, difficult to investigate issues. This pull request attempts to extend CKAN's debugging capabilities in specific areas to make it possible to make progress on those issues. It also addresses a few very small or simple problems that came up.

Debian dependency on webrequest

A user reported in a random comment on unrelated issue #1817 that the .deb package should depend on the webrequest package. This is now added to debian/control.in.

Remove unnecessary async keyword

installFromckanToolStripMenuItem_Click is marked as async, but it doesn't need to be, which causes a warning to be raised at compile time. This is now removed.

Fixes #2376.

Include webexception info in failure message for repo updates

#2391 contains multiple attestations that refreshing the registry sometimes fails, possibly related to how long CKAN has been running. AWebException is thrown when this happens, but its details are discarded.

Since the cause of this is unclear, we now add the exception to the error output. This is not a fix but may allow us to discover how to create a fix.

Suppress exceptions in OnPaint and skip Mono 3 workaround in Mono 5

A user reports in #2396 that the CKAN window turns into a big red X sometimes on MacOSX. This is apparently something WinForms does when you throw an exception in a control's OnPaint method, which we have here:

CKAN/GUI/MainModList.cs

Lines 427 to 444 in a495f43

public class MainModListGUI : DataGridView
{
protected override void OnPaint(PaintEventArgs e)
{
//Hacky workaround for https://bugzilla.xamarin.com/show_bug.cgi?id=24372
if (Platform.IsMono && !Platform.IsMonoFour)
{
var first_row_index = typeof (MainModListGUI).BaseType
.GetField("first_row_index", BindingFlags.NonPublic | BindingFlags.Instance);
var value = (int) first_row_index.GetValue(this);
if (value < 0 || value >= Rows.Count)
{
first_row_index.SetValue(this, 0);
}
}
base.OnPaint(e);
}

That was originally done to work around a bug in Mono 3 that was fixed in Mono 4. The idea was that the code would use the IsMonoFour check to avoid running on versions where the bug was fixed. However, now that we are mostly on Mono 5, IsMonoFour is no longer an adequate check; since Mono 5 is not Mono 4, this Mono 3 workaround has come back to life and is running on Mono 5.

Now all of that code is wrapped in a try/catch block that discards all the exceptions, and IsMonoFour is replaced with IsMonoFourOrLater to reflect the real intent of this code, preventing the Mono 3 workaround from running on Mono 5.

May or may not fix #2396.

More debug messages around lock acquisition and registry creation

KSP-CKAN/NetKAN#6459 describes an issue in which CKAN "has stopped working" immediately on launch, even if you just run ckan scan from the command line. When we tried capturing the --debug output, the last message was

log.DebugFormat("Preparing to load registry at {0}", directory);

In a working install, the next message would be one of these:

log.InfoFormat("Loaded CKAN registry at {0}", path);

log.InfoFormat("Creating new CKAN registry at {0}", path);

So something between those statements is somehow causing a crash. There's a lot of code in that gap. This pull request adds more DebugFormat statements to log all those various steps so we can narrow down what's happening.

Not a fix for KSP-CKAN/NetKAN#6459, but should help us to figure out what's going on with that issue.

@HebaruSan HebaruSan added Pull request Bug Easy This is easy to fix Core (ckan.dll) Issues affecting the core part of CKAN macOS Issues specific for macOS Network Issues affecting internet connections of CKAN and removed Pull request labels Apr 8, 2018
@politas politas merged commit 98cf765 into KSP-CKAN:master Apr 11, 2018
politas added a commit that referenced this pull request Apr 11, 2018
@politas politas removed Bug Network Issues affecting internet connections of CKAN Pull request labels Apr 11, 2018
@HebaruSan HebaruSan deleted the fix/debuggability branch April 11, 2018 15:50
@HebaruSan
Copy link
Member Author

@politas, this would be a decent time to release a 1.24.1 version, in my opinion. The most-encountered bugs in 1.24.0 are fixed, and we don't have very many larger (=higher-risk) changes yet.

I'd be willing to try my hand at preparing such a release if you agree it should happen but would like someone else to take care of it. Just say the word.

@politas
Copy link
Member

politas commented Apr 13, 2018

Next release has to be 1.25.0, as we have a spec change as part of the changes. I was thinking it was about time.

@politas
Copy link
Member

politas commented Apr 13, 2018

I have free time tomorrow, so I'll push it out then, if no one complains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Easy This is easy to fix macOS Issues specific for macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ckan Mac version show a cross after "recommended mods" Build Warning async method
2 participants