Add filename to error message #151
Conversation
| end | ||
|
|
||
| def raise_exception_if_encoding_failed(encode) | ||
| return unless encode.failed? | ||
| raise StandardError.new("Encoding failed: #{encode.errors.join(' ; ')}") | ||
| raise StandardError.new("Encoding failed for #{source_path}: #{encode.errors.join(' ; ')}") |
There was a problem hiding this comment.
I wonder if this is a good opportunity to create our own subclasses of StandardError (or, maybe RuntimeError) to carry this information consistently and in a machine-readable way?
There was a problem hiding this comment.
@cbeer Do you mean: keep the same error message, but just give the error class a more distinct name (such as EncodingCancelled)? Or do you mean that the error class should have more info than just the error message?
There was a problem hiding this comment.
Yes, I think this would benefit from our own subclass of exception, and that subclass can take the source path as an argument (something like:
class EncodingException < RuntimeError
def initialize(source_path, message)
...
end
end
) and use that both to build the message, but also provide the callers with something they could reasonably expect to handle.
|
Changes Unknown when pulling 23d79e9 on add_filename_to_error_message into ** on active_encode_dev_con**. |
val99erie
force-pushed
the
add_filename_to_error_message
branch
from
23d79e9
to
37c4991
Compare
|
I rebased to get those rubocop changes |
1 similar comment
| # while(encode.reload.running?) { sleep 10 } | ||
|
|
||
| raise_exception_if_encoding_failed(encode) | ||
| raise_exception_if_encoding_cancelled(encode) | ||
| raise ActiveEncodeError.new(encode.state, source_path, encode.errors) unless encode.completed? |
There was a problem hiding this comment.
Will this always throw an error since we're not blocking until the job is finished yet? Seems like it will always have a status of :running when it gets to this point.
There was a problem hiding this comment.
As far as I know, that encoding on line 14 can never work. That isn't the correct method signature, is it? I have code on my other branch that creates the encode differently and waits for it to finish running.
There was a problem hiding this comment.
I think it is okay, but maybe we can just forget about that point right now. Assuming the encode job is blocking then I think the unless completed check is correct.
|
I don't know what to do to make the coveralls build pass. |
|
@val99erie I changed the coveralls config for this repo to only fail is the coverage decrease threshold exceeds 0.3%, which is a value we use elsewhere in Hydralandia. |
Because I'm submitting 2 PRs in a row against the same branch, the diff for this PR will also include the changes for PR #150 until PR #150 gets merged.
The true diff for this PR is quite small. You can see it here:
cancelled_status...add_filename_to_error_message
Please merge PR #150 before this one. Thanks!