Block Name In Errors

[gabe-l-hart]

Description

This PR adds try/except handling around the core loop in Pipeline. In the handler clause, it creates a wrapper exception, attempting to match the type of the internal error, with the block_name and block_type included.

Closes: #128

[russellb]

one suggestion to localize the try/except handling to only the parts calling block-specific code.

Also, do you have an example of what an error looked like before and after the change?

[russellb]

It looks like these lines capture the block-specific code, so you could localize your addition to this part, I think.

[gabe-l-hart]

Good point! Do you think it's worth containing the config parsing as well? It seems like the use of direct indexing (vs .get) for "name", "type", and "config" implies that these are all required fields, but I don't see anywhere that they're validated (though that might happen elsewhere that I haven't found yet?). If it's not done elsewhere, it may be worth doing that validation in the __init__ to avoid getting part-way through the expensive pipeline operation before discovering configuration errors.

[russellb]

we now have some config validation, at least for the configs we have in the tree. See make validate-pipelines. It uses a jsonschema definition

[gabe-l-hart]

Ok, cool, makes sense!

[gabe-l-hart]

Thanks for the suggestion! I've localized the error handling to just the generate call.

Also, do you have an example of what an error looked like before and after the change?

I don't have a concrete example of before/after, but this uses raise ... from ... syntax. The parent exception will attempt to match the exception type of the one raised in the block with a message "BLOCK ERROR [{block_type class name}/{block_name}]: {child exc message}". The original exception can be accessed via the __cause__ attribute of the wrapper exception.

[markmc]

Also, do you have an example of what an error looked like before and after the change?

Here's what an exception from LLMBlock.generate() looks like:

INFO 2024-07-17 07:08:14,120 pipeline.py:77: generate Running block: gen_questions
INFO 2024-07-17 07:08:14,120 pipeline.py:78: generate Dataset({
    features: ['task_description', 'seed_question', 'seed_response'],
    num_rows: 5
})
Traceback (most recent call last):
...
  File "/home/markmc/sdg/src/instructlab/sdg/sdg.py", line 20, in generate
    dataset = pipeline.generate(dataset)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/markmc/sdg/src/instructlab/sdg/pipeline.py", line 80, in generate
    dataset = block.generate(dataset)
              ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/markmc/sdg/src/instructlab/sdg/llmblock.py", line 181, in generate
    raise Exception("TEST")
Exception: TEST

This isn't actually so problematic since we log which block is being executed, so the user has some context to use in order to debug the error

The example I gave in #128 is failing to load a config file when instantiating a block - an error with the pipeline definition, but also one that can't be detected by the schema

[markmc]

nit: underscore prefix this if you want just .block_name and .block_type to be used?

[gabe-l-hart]

I left that one "public" thinking that some might like to be able to actually muck with the block itself when handling the exception.

[markmc]

Thanks, @gabe-l-hart !