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

No Header Is Written When Exception Occurs While Writing Inside a Block #5

Closed
jstrait opened this issue Feb 12, 2013 · 6 comments
Closed
Labels

Comments

@jstrait
Copy link
Owner

jstrait commented Feb 12, 2013

For example:

Writer.new("error.wav", format) do |writer|
  writer.write(buffer)
  x = 1 / 0
end

When this is run, the error.wav will exist on disk, but will not be playable because the Writer was never closed.

Adding an ensure clause to the block_given? section of Writer.new should resolve this.

@jamestunnell
Copy link
Contributor

I'll work on this issue, but I don't think I have permissions to assign it to myself.

@jamestunnell
Copy link
Contributor

I committed changes to address this issue on my fork: https://github.com/jamestunnell/wavefile

You can pull in the changes if you think they take care of the issue (tests are there as well).

@jamestunnell
Copy link
Contributor

The exact commit is jamestunnell@8e75345814

@jstrait
Copy link
Owner Author

jstrait commented Feb 15, 2013

Thanks for the quick response!

Took a look at the commit. Thanks for the well documented and easy to read tests. Only comment is around the test_exception_without_block test. When not using a block, it's the client's responsibility to make sure the Writer gets closed, which can be done using begin/ensure for example. The situation in this test of the header not being written could be resolved by closing the file in an ensure block. So it sort of seems like it's testing that if the client does the wrong thing, they get the wrong result.

What it ultimately seems to be testing is that if the file hasn't been closed yet, you won't be able to read data out of it. It could possibly be simplified by removing the begin/rescue and ZeroDivisionError, and just reading the file before the Writer is closed:

writer = Writer.new("#{OUTPUT_FOLDER}/exception_without_block.wav", format)
writer.write(Buffer.new([1, 2, 3, 4], format))

reader = Reader.new("#{OUTPUT_FOLDER}/exception_without_block.wav")
assert_equal(0, reader.samples_remaining)

@jamestunnell
Copy link
Contributor

Yeah, I guess that test is a bit pointless. Not hard to take it out though!

@jstrait
Copy link
Owner Author

jstrait commented Feb 16, 2013

Awesome, thanks! I've merged in your commits, so closing the issue.

@jstrait jstrait closed this as completed Feb 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants