Skip to content

Add support for error logging (e.g. bad font instance keys) to be added to the renderer errors list. #1678

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

Open
glennw opened this issue Sep 7, 2017 · 5 comments

Comments

@glennw
Copy link
Member

glennw commented Sep 7, 2017

No description provided.

@glennw
Copy link
Member Author

glennw commented Oct 4, 2017

There's a couple of ways I can see to do this:

  1. Similar to the Vec<RendererError> result that the render function provides, we could add a Vec<FrameError> result to the update method in WR. In this case any errors that occur during building a frame would be accumulated into a vector, passed with the frame to the Renderer object and retrieved by the client code when calling update.

  2. Add a trait / callback to RendererOptions which is an error handling callback. This would result in a user callback being invoked on an arbitrary thread (typically render backend thread, but potentially worker threads too) for each error.

These are for errors that you typically want to assert on during development etc, so they are a bit different than the issue to make logging available to WR. Alternatively, we could decide we don't need this and just support logging infrastructure only.

Any preferences on how this would be implemented @jrmuizel @nical @kvark ?

@kvark
Copy link
Member

kvark commented Oct 4, 2017

I'd slightly prefer to route this via log crate and capture the output by a specialized handler on some machines running Gecko, so that it can be appended to other logs/crash reports. The severity of capturing could be configured naturally, and writing/managing those messages would be trivial by people who work with WR code.

@glennw
Copy link
Member Author

glennw commented Oct 4, 2017

My only concern is that @jrmuizel expressed that in this particular case (a font instance failing to exist) he'd want to assert in a debug build, and it's very easy to miss an error log during development, compared to an assert.

@kvark
Copy link
Member

kvark commented Oct 4, 2017

@glennw technically, nothing would stop our log handler to assert in place under specific conditions (debug build, message severity, settings, etc). Bonus points comparing to Vec<Error> is that such an assert would produce a valid stack trace.

Edit: on a second thought, I'm not sure about the stack

@glennw
Copy link
Member Author

glennw commented Oct 4, 2017

That's a good point about the stack trace, though - it'd be handy to have that when the error occurs. Which probably makes me lean towards option (2) above or just using logging, like you said, if that works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants