debugging - avoid Project View crash by displaying default project image in case of problems

[trevelyan]

It's possible to programmatically create project files without images. The resulting files load fine right now when they are imported into Lighthouse, but the application crashes when it tries to display them on the project page.

This just avoids the interface crashing by loading the default-cover-image in cases where there is something wrong with the image in the project file.

[mikehearn]

Thanks!

Does a missing optional byte field really create an empty byte array? I'd have thought this would yield a null return. At any rate, I think there is a better fix: Project.getCoverImage() should return null if there is no image (newly marked as nullable), it can test the protobuf using getExtraDetails().hasCoverImage(). Then ProjectView should test the return against null and use the default image in that case. If the image is unloadable or corrupted rather than just missing, then I think it's OK to crash the app as this way we can get crash reports and learn about bugs in image decoding.

[mikehearn]

Also, if you can squash your commits for things like formatting fixes that's helpful. You can use git rebase then do a force push. Otherwise no worries, I'll do it when I merge.

[trevelyan]

Hey Mike,

Screenshot attached of the program crashing without the edit....

Just as background, I've been slowly experimenting with some command-line
programs along the lines of the /examples folder in bitcoinj. Goal is
teaching myself the system by putting together some one-off tools that
might help the general public. I figure at the least this may produce some
example code that can be useful to other programmers more accustomed to cli
on linux/mac.

For maximum usability, I'd suggest most data in the project file should be
completely optional. In the absence of that perhaps the ProjectModel can
auto-add the missing content before export (i.e. refuse to produce a
.lighthouse-project that will fail to load).

Best,

--david

On Thu, Jan 29, 2015 at 10:08 PM, Mike Hearn [email protected]
wrote:

Thanks!

Does a missing optional byte field really create an empty byte array? I'd
have thought this would yield a null return. At any rate, I think there is
a better fix: Project.getCoverImage() should return null if there is no
image (newly marked as nullable), it can test the protobuf using
getExtraDetails().hasCoverImage(). Then ProjectView should test the return
against null and use the default image in that case. If the image is
unloadable or corrupted rather than just missing, then I think it's OK to
crash the app as this way we can get crash reports and learn about bugs in
image decoding.

—
Reply to this email directly or view it on GitHub
#146 (comment).

[mikehearn]

I believe you that it crashes, no need for a screenshot :)

The command line tools sound good to me. I'd like to see more ways to handle Lighthouse projects programmatically and cli tools are a good start.

I think most project data already is optional, as defined at the protobuf level. That's why it crashes - the file format defines the image as optional but I forgot to handle that case at the UI level as the app never generates such a project. So we just need to bubble that up through the object model and then do what this patch does. My feedback is really quite minor - just saying treat "image is missing" differently to "image is corrupted".

[trevelyan]

Added loadFromFile(File f) method to PledgingWallet class. It would be useful to hide this complexity.

Apologies for double-sending the earlier requests. I am new to git and there seems to be no easy way to send a single commit.

[mikehearn]

Ah ha - the idea is, you create a branch for each feature. Otherwise as you can see your latest commit gets rolled into this pull request even though they are logically separate. Git can be a bit fiddly at first but when you get used to it, it's quite powerful.

The command to master is called rebase. It lets you edit history by removing/adding/re-arranging commits. You could use it to split this latest commit out of your pre-existing master branch and onto a 'feature branch', which you can then push and open a pull req for.

Or if you prefer you can just do things the old fashioned way and email .patch files to the mailing list :)

In the latest commit, watch out for:

1. Whitespace/formatting. Try and match the style you see around you when coding.
2. You're catching and rethrowing IO errors, but still declaring them in the throws clause. It's probably OK to just let them propagate out, in which case you don't need the try/catch.
3. Unit test to verify the method works would be good!