Import/Export IO: Handle errors in a more consistent manner #117881

New Issue

Jesse Yurkovich · 2024-02-06T08:33:39+01:00

Jesse Yurkovich commented

2024-02-06 08:33:39 +01:00

Issue

Our file import / export code is quite haphazard about handling and reporting certain types of filepath errors, and errors in general. As part of the Collection IO project it will be important for misconfigured paths and other errors to report consistently, and certainly not crash, when invoking Collection Export from either the UI or from Python.

This task here tracks trying to unify things with considerations for all of the following:

Crashes need fixed
Should each of these scenarios "Report" in some manner?
- If so, should there be a common format to the message at all?
Should we align the Python exceptions at all?

Proposal

Use Reports + OPERATION_CANCELLED to indicate failure
- Failure to perform the import/export should be a user-visible, and user-understandable, "Report" in addition to a non-success return code
- Keep the Reports succinct. Importers and Exporters are free to use CLOG for general purpose logging and additional failure details as necessary.
Use a common message format
- The Report should say which format was affected e.g. prefix it with "USD Export:" or "OBJ Export:" etc. Rationale comes from the Collections IO project. Multiple exporters may be configured on a collection and it should be possible to determine which one is failing. The Collections IO code itself will not "Report" because it could lead to a "double report" situation.
- If paths appear in Reports or in CLOG, surround them with single quotes '
As much as possible, align Python exception types
Disallow whitespace-only file names like .glb
Allow paths with no extensions e.g. /some/path/could_be_an_obj_or_maybe_a_stl
- Blender's drag-drop support depends on file extensions to work; it would allow producing files that won't work there, but I don't have an example of other software that prevents this situation.

Status

Below is what the current state of the world looks like for Exporters

Alembic
bpy.ops.wm.alembic_export(filepath="")                      # ok: Report + CANCELLED 
bpy.ops.wm.alembic_export(filepath=" ")                     # ok: Report + CANCELLED 
bpy.ops.wm.alembic_export(filepath="t:/")                   # ok: Report + CANCELLED 
bpy.ops.wm.alembic_export(filepath="t:/noext_abc")          # ok: FINISHED 
bpy.ops.wm.alembic_export(filepath="x:/nonexistent.abc")    # ok: Report + CANCELLED 
bpy.ops.wm.alembic_export(filepath="<:invalid:>")           # ok: Report + CANCELLED 

USD
bpy.ops.wm.usd_export(filepath="")                      # ok: Report + python RuntimeError (message is confusing)
bpy.ops.wm.usd_export(filepath=" ")                     # ok: Report + python RuntimeError (message is confusing)
bpy.ops.wm.usd_export(filepath="t:/")                   # ok: Report + python RuntimeError (message is confusing)
bpy.ops.wm.usd_export(filepath="t:/noext_usd")          # ok: USD needs an extension. Report + python RuntimeError (message is confusing)
bpy.ops.wm.usd_export(filepath="x:/nonexistent.usd")    # ok: Report + python RuntimeError (message is confusing)
bpy.ops.wm.usd_export(filepath="<:invalid:>")           # ok: Report + python RuntimeError (message is confusing)

OBJ
bpy.ops.wm.obj_export(filepath="")                      # bug: FINISHED - nothing output
bpy.ops.wm.obj_export(filepath=" ")                     # bug: FINISHED - nothing output 
bpy.ops.wm.obj_export(filepath="t:/")                   # bug: FINISHED - nothing output
bpy.ops.wm.obj_export(filepath="t:/noext_obj")          # ok: FINISHED 
bpy.ops.wm.obj_export(filepath="x:/nonexistent.obj")    # bug: FINISHED - nothing output
bpy.ops.wm.obj_export(filepath="<:invalid:>")           # bug: FINISHED - nothing output

STL
bpy.ops.wm.stl_export(filepath="")                      # crash
bpy.ops.wm.stl_export(filepath=" ")                     # crash
bpy.ops.wm.stl_export(filepath="t:/")                   # crash
bpy.ops.wm.stl_export(filepath="t:/noext_stl")          # ok: FINISHED
bpy.ops.wm.stl_export(filepath="x:/nonexistent.stl")    # crash
bpy.ops.wm.stl_export(filepath="<:invalid:>")           # crash

PLY
bpy.ops.wm.ply_export(filepath="")                      # crash
bpy.ops.wm.ply_export(filepath=" ")                     # crash
bpy.ops.wm.ply_export(filepath="t:/")                   # crash
bpy.ops.wm.ply_export(filepath="t:/noext_ply")          # ok: FINISHED
bpy.ops.wm.ply_export(filepath="x:/nonexistent.ply")    # crash
bpy.ops.wm.ply_export(filepath="<:invalid:>")           # crash

FBX
bpy.ops.export_scene.fbx(filepath="")                   # python Exception
bpy.ops.export_scene.fbx(filepath=" ")                  # python Exception
bpy.ops.export_scene.fbx(filepath="t:/")                # python PermissionError
bpy.ops.export_scene.fbx(filepath="t:/noext_fbx")       # ok: FINISHED
bpy.ops.export_scene.fbx(filepath="x:/nonexistent.fbx") # python FileNotFoundError
bpy.ops.export_scene.fbx(filepath="<:invalid:>")        # python FileNotFoundError

GLTF
bpy.ops.export_scene.gltf(filepath="")                      # python FileNotFoundError
bpy.ops.export_scene.gltf(filepath=" ")                     # bug??: FINISHED --> succeeded in writing a " .glb" file
bpy.ops.export_scene.gltf(filepath="t:/")                   # python PermissionError
bpy.ops.export_scene.gltf(filepath="t:/noext_gltf")         # ok: FINISHED
bpy.ops.export_scene.gltf(filepath="x:/nonexistent.gltf")   # python FileNotFoundError
bpy.ops.export_scene.gltf(filepath="<:invalid:>")           # python FileNotFoundError

### Issue Our file import / export code is quite haphazard about handling and reporting certain types of `filepath` errors, and errors in general. As part of the Collection IO project it will be important for misconfigured paths and other errors to report consistently, and certainly not crash, when invoking Collection Export from either the UI or from Python. This task here tracks trying to unify things with considerations for all of the following: * Crashes need fixed * Should each of these scenarios "Report" in some manner? * If so, should there be a common format to the message at all? * Should we align the Python exceptions at all? ### Proposal - Use Reports + OPERATION_CANCELLED to indicate failure - Failure to perform the import/export should be a user-visible, and user-understandable, "Report" in addition to a non-success return code - Keep the Reports succinct. Importers and Exporters are free to use CLOG for general purpose logging and additional failure details as necessary. - Use a common message format - The Report should say which format was affected e.g. prefix it with "USD Export:" or "OBJ Export:" etc. Rationale comes from the Collections IO project. Multiple exporters may be configured on a collection and it should be possible to determine which one is failing. The Collections IO code itself will not "Report" because it could lead to a "double report" situation. - If `paths` appear in Reports or in CLOG, surround them with single quotes `'` - As much as possible, align Python exception types - Disallow whitespace-only file names like ` .glb` - Allow paths with no extensions e.g. `/some/path/could_be_an_obj_or_maybe_a_stl` - Blender's drag-drop support depends on file extensions to work; it would allow producing files that won't work there, but I don't have an example of other software that prevents this situation. ### Status Below is what the current state of the world looks like for Exporters ``` Alembic bpy.ops.wm.alembic_export(filepath="") # ok: Report + CANCELLED bpy.ops.wm.alembic_export(filepath=" ") # ok: Report + CANCELLED bpy.ops.wm.alembic_export(filepath="t:/") # ok: Report + CANCELLED bpy.ops.wm.alembic_export(filepath="t:/noext_abc") # ok: FINISHED bpy.ops.wm.alembic_export(filepath="x:/nonexistent.abc") # ok: Report + CANCELLED bpy.ops.wm.alembic_export(filepath="<:invalid:>") # ok: Report + CANCELLED USD bpy.ops.wm.usd_export(filepath="") # ok: Report + python RuntimeError (message is confusing) bpy.ops.wm.usd_export(filepath=" ") # ok: Report + python RuntimeError (message is confusing) bpy.ops.wm.usd_export(filepath="t:/") # ok: Report + python RuntimeError (message is confusing) bpy.ops.wm.usd_export(filepath="t:/noext_usd") # ok: USD needs an extension. Report + python RuntimeError (message is confusing) bpy.ops.wm.usd_export(filepath="x:/nonexistent.usd") # ok: Report + python RuntimeError (message is confusing) bpy.ops.wm.usd_export(filepath="<:invalid:>") # ok: Report + python RuntimeError (message is confusing) OBJ bpy.ops.wm.obj_export(filepath="") # bug: FINISHED - nothing output bpy.ops.wm.obj_export(filepath=" ") # bug: FINISHED - nothing output bpy.ops.wm.obj_export(filepath="t:/") # bug: FINISHED - nothing output bpy.ops.wm.obj_export(filepath="t:/noext_obj") # ok: FINISHED bpy.ops.wm.obj_export(filepath="x:/nonexistent.obj") # bug: FINISHED - nothing output bpy.ops.wm.obj_export(filepath="<:invalid:>") # bug: FINISHED - nothing output STL bpy.ops.wm.stl_export(filepath="") # crash bpy.ops.wm.stl_export(filepath=" ") # crash bpy.ops.wm.stl_export(filepath="t:/") # crash bpy.ops.wm.stl_export(filepath="t:/noext_stl") # ok: FINISHED bpy.ops.wm.stl_export(filepath="x:/nonexistent.stl") # crash bpy.ops.wm.stl_export(filepath="<:invalid:>") # crash PLY bpy.ops.wm.ply_export(filepath="") # crash bpy.ops.wm.ply_export(filepath=" ") # crash bpy.ops.wm.ply_export(filepath="t:/") # crash bpy.ops.wm.ply_export(filepath="t:/noext_ply") # ok: FINISHED bpy.ops.wm.ply_export(filepath="x:/nonexistent.ply") # crash bpy.ops.wm.ply_export(filepath="<:invalid:>") # crash FBX bpy.ops.export_scene.fbx(filepath="") # python Exception bpy.ops.export_scene.fbx(filepath=" ") # python Exception bpy.ops.export_scene.fbx(filepath="t:/") # python PermissionError bpy.ops.export_scene.fbx(filepath="t:/noext_fbx") # ok: FINISHED bpy.ops.export_scene.fbx(filepath="x:/nonexistent.fbx") # python FileNotFoundError bpy.ops.export_scene.fbx(filepath="<:invalid:>") # python FileNotFoundError GLTF bpy.ops.export_scene.gltf(filepath="") # python FileNotFoundError bpy.ops.export_scene.gltf(filepath=" ") # bug??: FINISHED --> succeeded in writing a " .glb" file bpy.ops.export_scene.gltf(filepath="t:/") # python PermissionError bpy.ops.export_scene.gltf(filepath="t:/noext_gltf") # ok: FINISHED bpy.ops.export_scene.gltf(filepath="x:/nonexistent.gltf") # python FileNotFoundError bpy.ops.export_scene.gltf(filepath="<:invalid:>") # python FileNotFoundError ```

Jesse Yurkovich added the

Module

Pipeline, Assets & IO

Type

Design

labels 2024-02-06 08:33:40 +01:00

Jesse Yurkovich referenced this issue

2024-02-06 08:36:32 +01:00

General IO: Persistent Export settings on Collections #115690

Bastien Montagne commented

2024-02-22 10:39:56 +01:00

Generally LGTM.

The Report should say which format was affected e.g. prefix it with "USD Export:" or "OBJ Export:" etc. Rationale comes from the Collections IO project. Multiple exporters may be configured on a collection and it should be possible to determine which one is failing. The Collections IO code itself will not "Report" because it could lead to a "double report" situation.

I wonder if the WM code should not always do that (prepend the operator's name to all of its reports)... But maybe for now such a convention is simpler to implement and less disruptive in other areas.

Generally LGTM. > - The Report should say which format was affected e.g. prefix it with "USD Export:" or "OBJ Export:" etc. Rationale comes from the Collections IO project. Multiple exporters may be configured on a collection and it should be possible to determine which one is failing. The Collections IO code itself will not "Report" because it could lead to a "double report" situation. I wonder if the WM code should not always do that (prepend the operator's name to all of its reports)... But maybe for now such a convention is simpler to implement and less disruptive in other areas.

Jesse Yurkovich referenced this issue

2024-02-25 21:28:59 +01:00

Fix: Proper error handling for STL, PLY, and OBJ IO #118731

Jesse Yurkovich referenced this issue from a commit

2024-02-26 20:45:55 +01:00

Fix: Proper error handling for STL, PLY, and OBJ IO

Sign in to join this conversation.