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

Moving error formatting from wgpu-rs to wgpu-core #1082

Closed
scoopr opened this issue Dec 8, 2020 · 1 comment
Closed

Moving error formatting from wgpu-rs to wgpu-core #1082

scoopr opened this issue Dec 8, 2020 · 1 comment
Labels
area: validation Issues related to validation, diagnostics, and error handling help required We need community help to make this happen. type: enhancement New feature or request

Comments

@scoopr
Copy link
Contributor

scoopr commented Dec 8, 2020

As discussed in #1068 it may be that some of the error formatting that is now in wgpu-rs should in fact be moved to wgpu-core.

As I don't have time to do help with it right now, I wanted to write down the steps I was thinking about it so that I don't forget. Also maybe it helps if anyone else is willing to help with this one.

The format_error in wgpu-rs error.rs is the one that should end up either in directly to Global, or as a separate helper that takes Global as argument (It is basically doing that through Context now). Trivial in itself, but it depend on fmt_pretty_any

fmt_pretty_any is what is doing the downcasting of errors, and mostly could just be moved to wgpu-core as-is, but it also downcasts the ContextError that is more specific to wgpu-rs.

The ContextError is used to decorate the api calls in wgpu-rs for which api call the error came from. This was initially done because it had convenient access to the descriptor label, even when nothing was created on errors, as the error Ids were created on wgpu-rs error side, but since this was changed in #1034, it does not need to be in wgpu-rs side anymore.

So working backwards,
• Move/add ContextError to wgpu-core
• Decorate the api call sites in wgpu-core
• Remove the ContextError decoration in wgpu-rs handle_error
• Now all of error.rs could be moved in wgpu-core (and slight tweak to handle_error to call the format_error in its new place)

Alternatively, if just having some working format_error is deemed more important and waffing about the ContextError is boring busywork to be done later,
• Move PrettyError trait and its impls (except for the ContextError)
• Add some hook (callback?) to format_error/fmt_pretty_any that allows wgpu-rs to inject its error to be downcasted
• Move format_error to core. That should leave only the ContextError related stuff in error.rs

That was quite a lot of text, what is ultimately fairly simple set of changes.

@kvark kvark added area: validation Issues related to validation, diagnostics, and error handling help required We need community help to make this happen. type: enhancement New feature or request labels Dec 8, 2020
bors bot added a commit that referenced this issue Aug 3, 2021
1728: Move error formatting functionality  r=kvark a=scoopr

**Connections**
This is an attempt to implement #1082 

**Description**
This is a fairly straightforward move of the error formatting that were in wgpu-rs in to wgpu-code.

The enriching of some of the errors with the additional `ContextError` were still left with the wgpu-rs code, as I realised the current error messages refer to the API names as they appear in rust code and might not make sense for all the consumers of wgpu-core. It could be left as is and other consumers might enrich in their own ways, or the messages could be reformatted to be more conversational

**Testing**
Limited testing with the examples


Co-authored-by: Mikko Lehtonen <[email protected]>
@scoopr
Copy link
Contributor Author

scoopr commented Aug 3, 2021

Done in #1728

@scoopr scoopr closed this as completed Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: validation Issues related to validation, diagnostics, and error handling help required We need community help to make this happen. type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants