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

Use gr.Error for errors in gr.Audio #6335

Closed
1 task done
hysts opened this issue Nov 8, 2023 · 7 comments · Fixed by #6672
Closed
1 task done

Use gr.Error for errors in gr.Audio #6335

hysts opened this issue Nov 8, 2023 · 7 comments · Fixed by #6672
Assignees
Labels
bug Something isn't working

Comments

@hysts
Copy link
Collaborator

hysts commented Nov 8, 2023

  • I have searched to see if a similar issue already exists.

Would it be possible to catch Exception and re-raise it as gr.Error?

I was checking the recently added min_length and max_length of gr.Audio with code like this:

import gradio as gr

gr.Interface(fn=lambda x: x, inputs=gr.Audio(min_length=5, max_length=60), outputs=gr.Audio()).queue().launch()

and realized that it raises ValueError in .preprocess() when one feeds an audio that is too short/long.

if self.min_length is not None and duration < self.min_length:
raise ValueError(
f"Audio is too short, and must be at least {self.min_length} seconds"
)
if self.max_length is not None and duration > self.max_length:
raise ValueError(
f"Audio is too long, and must be at most {self.max_length} seconds"
)

But the problem is that the error is only shown in your terminal. In the case of HF Spaces, users except for the owner of the Space cannot see the log, and it makes it hard for them to understand why they get errors.

This reminds me that I wished ValueError, RuntimeError etc. would be shown as gr.Error many times in the past. I've mentioned gr.Audio as an example above, but more generally, I think it would be nice if all errors were caught and re-raised as gr.Error.

@abidlabs
Copy link
Member

abidlabs commented Nov 8, 2023

Thanks @hysts! I agree -- let's change these ValueError to gr.Error -- I thought we had already made that change actually @hannahblair?

In terms of raising all errors to the frontend, we wouldn't want to do that by default because it might expose sensitive bits of the code for the gradio app to users, but if you are certain you want to do that, you can set show_error=True in launch()

@abidlabs abidlabs added the bug Something isn't working label Nov 8, 2023
@hysts
Copy link
Collaborator Author

hysts commented Nov 8, 2023

Oh, I wasn't aware of the show_error argument. Thanks!

@abidlabs abidlabs changed the title Catch exceptions and re-raise as gr.Error Use gr.Error for errors in gr.Audio Nov 9, 2023
@abidlabs
Copy link
Member

This should be fixed in 4.2.0!

@hysts
Copy link
Collaborator Author

hysts commented Dec 4, 2023

@abidlabs @hannahblair
Looks like it's still raising ValueError.

raise ValueError(
f"`sources` must be a list consisting of elements in {valid_sources}"
)
for source in self.sources:
if source not in valid_sources:
raise ValueError(
f"`sources` must a list consisting of elements in {valid_sources}"
)
valid_types = ["numpy", "filepath"]
if type not in valid_types:
raise ValueError(
f"Invalid value for parameter `type`: {type}. Please choose from one of: {valid_types}"
)
self.type = type
self.streaming = streaming
if self.streaming and "microphone" not in self.sources:
raise ValueError(
"Audio streaming only available if sources includes 'microphone'."
)

@hysts hysts reopened this Dec 4, 2023
@abidlabs
Copy link
Member

abidlabs commented Dec 4, 2023

Hey @hysts, these are developer-facing errors (e.g. something is wrong with the code itself) so they should just raise a ValueError which would be shown to the developer at runtime.

The user-facing gr.Error should only be raised if a user does something invalid in the Gradio app, like uploads a file that is too long.

OK to close this issue?

@hysts
Copy link
Collaborator Author

hysts commented Dec 4, 2023

@abidlabs Ah, so sorry. Yeah, I know. Somehow I linked completely different piece of code. 😇
I meant this:

if self.min_length is not None and duration < self.min_length:
raise ValueError(
f"Audio is too short, and must be at least {self.min_length} seconds"
)
if self.max_length is not None and duration > self.max_length:
raise ValueError(
f"Audio is too long, and must be at most {self.max_length} seconds"
)

@abidlabs
Copy link
Member

abidlabs commented Dec 4, 2023

Oh haha that's weird I thought we fixed it. Should be an easy fix anyways!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants