Improve logging when operator failed before generating artifact

[likawind]

Describe your changes and why you are making these changes

This PR improves logging when a operator failed without generating artifact metadata. The current situation being:

• operator failed without generating artifact metadata
• we attempt to upload artifact metadata
• we failed to read that metadata and thus result in showing a missing storage object failure in most error logs
  As the improvements, we modifies the logging with minimum change on current life-cycle management behavior:
• If artif metadata is failed to read, we will log the error rather than fail the function call. We expect orchestration to handle downstream execution based on operator's state.
  • when reading the metadata, we assume empty metadata path + metadata field implies empty metadata, rather than an error state.
• In orchestrator, we log operator's failure reason together with operator's ID when wf execution is stopped due to operator failures.

Related issue number (if any)

ENG-2815

Testings

We are not expecting to change any life-cycle management behavior. I'd consider it as success as long as all integration tests passed

Checklist before requesting a review

• I have created a descriptive PR title. The PR title should complete the sentence "This PR...".
• I have performed a self-review of my code.
• I have included a small demo of the changes. For the UI, this would be a screenshot or a Loom video.
• If this is a new feature, I have added unit tests and integration tests.
• I have run the integration tests locally and they are passing.
• I have run the linter script locally (See python3 scripts/run_linters.py -h for usage).
• All features on the UI continue to work correctly.
• Added one of the following CI labels:
  • run_integration_test: Runs integration tests
  • skip_integration_test: Skips integration tests (Should be used when changes are ONLY documentation/UI)

[kenxu95]

Curious, is this error ever expected in any cases? If it's a really fatal error we should probably also add a Tip to the execution state?

[kenxu95]

And make it a system error or something.

[likawind]

This issue is often triggered by the upstream op failed without creating the metadata file. I don't really want to update the exec status here since in most cases the artifact should be canceled, and caller decides. The main goal here is to prevent this from returning an err object to caller which often gives us the false signal on real failure (the op failure). So I think logging it in the context for now is fine

[likawind]

I can add a comment here

[kenxu95]

is execState.Error != nil in this case? I worry this will panic if there wasn't an issue with the computation, but something bad happened when we ReadFromStorage.

[likawind]

yeah that's a good catch, we should check the pointer state before assigning values

[kenxu95]

Nice, very clean!