daisy / pipeline-ui

A user interface for the DAISY Pipeline 2
MIT License
5 stars 2 forks source link

Improve speech service connection information #197

Closed marisademeglio closed 4 weeks ago

marisademeglio commented 4 months ago

The Pipeline's ability to connect to cloud based speech services (e.g. Azure, Google) can be impacted by issues related to internet connection or speech service credentials, but it is currently difficult to troubleshoot for a user of the Pipeline app because not enough information is reported.

The log created by the Pipeline engine does not report why a speech service is not working, just that it is available or unavailable. The app currently can only report whether the service is connected or not by inferring from the voices list given by the engine's /voices endpoint which services were included.

bertfrees commented 4 months ago

Do we need another endpoint for the TTS engines? For each engine we could list:

bertfrees commented 4 months ago

Thinking about what would be a good name for the endpoint. Perhaps /tts-engines.

bertfrees commented 4 months ago

It would already be an improvement to include more details in the log.

avneeshsingh commented 3 months ago

This issue is faced by some organizations in India also. Please give it higher priority for next release.

marisademeglio commented 1 month ago

It would be helpful to see a sketch of what info would be returned by /tts-engines, even a very basic version of the endpoint response.

bertfrees commented 1 month ago

Some examples:

<tts-engines xmlns="http://www.daisy.org/ns/pipeline/data" href="http://localhost:8181/ws/tts-engines">
    <tts-engine name="osx-speech" status="available" voices="http://localhost:8181/ws/voices?engine=osx-speech"/>
    <tts-engine name="google" status="error" message="Failed to retreive voices: API key not valid. Please pass a valid API key."/>
    <tts-engine name="azure" status="disabled" message="Property not set: org.daisy.pipeline.tts.azure.key"/>
</tts-engines>
<tts-engines xmlns="http://www.daisy.org/ns/pipeline/data" href="http://localhost:8181/ws/tts-engines">
    <tts-engine name="azure" status="error" message="Failed to retreive voices: Request canceled: Get voices list failed: Authentication error (401). Please check subscription information and region name."/>
</tts-engines>
NPavie commented 1 month ago

The new endpoint seems to work after @bertfrees update of the pipeline

I have the following message reported for Google TTS : Failed to retreive voices: Method doesn't allow unregistered callers (callers without established identity). Please use API Key or other form of API consumer identity to call this API. (I don't have any key specified)

For Azure TTS, if i disconnect with the UI button, i get the following error message : subscriptionKey

The section might need to become scrollable : with the Google TTS message, the element are overflowing after the frame

image

bertfrees commented 1 month ago

When no apikey is supplied, the value should be null, and in this case I raise a "Property not set" exception, which results in status="disabled". But probably the UI is sending an empty string. I will check for empty strings as well.

NPavie commented 1 month ago

For disconnection currently we are indeed sending an empty key to the engine.

Can a property be set to null with the current endpoint ? (like by setting the property without a value, or by using the delete method instead ?)

bertfrees commented 1 month ago

Aha, no, setting a null value is currently not possible.

However, what I forgot to mention is that Pipeline now automatically makes available a "org.daisy.pipeline.tts.<engine>.enabled" property for each TTS service.

NPavie commented 1 month ago

haaa cool ! I'll test that instead

marisademeglio commented 1 month ago

The section might need to become scrollable : with the Google TTS message, the element are overflowing after the frame

Can we change the message?

bertfrees commented 1 month ago

The full error that we get from Google is:

{
  "error": {
    "code": 403,
    "message": "Method doesn't allow unregistered callers (callers without established identity). Please use API Key or other form of API consumer identity to call this API.",
    "status": "PERMISSION_DENIED"
  }
}

The error message is passed on to the user as-is. For certain well-known errors we could consider omitting the long message and output a simpler message instead. E.g. we could check for "PERMISSION_DENIED" and output a simpler message. But I think it's a bit risky. We might miss essential information for troubleshooting connection issues, and that's why we're adding this feature after all, to pass as much information as possible on to the user, so that they don't have to go looking into the log files. Besides, I don't know if we can guarantee that error messages never exceed a certain number of characters this way.

marisademeglio commented 1 month ago

Maybe there's a way to show both a short and long error message, e.g. show the PERMISSION_DENIED simple message and then make the full message available if they want to see more info. Users won't know necessarily recognize that google message as a permissions issue either, the wording is really not plain language.

bertfrees commented 1 month ago

I agree we should find a solution in which we can enhance the messages from Google that might be too obscure, yet are able to provide all the information.

marisademeglio commented 4 weeks ago

From the slack chat

Messages are shortened/simplified, and if it is still too long, I change it into a generic message and move the original message to the details.

Is anything additional required in the UI?

bertfrees commented 4 weeks ago

Did Nico already make the error message expandable from the short version to the detailed one?

NPavie commented 4 weeks ago

@bertfrees @marisademeglio short and detailed messages are now added with a details element and the short message is marked as summary.

It creates an expandable section to display the long message in my tests, the only thing i'm not sure 100% is the accessibility of this element.

avneeshsingh commented 4 weeks ago

HTML details works well on browsers. Both NVDA and JAWS support it. But I have not tried it on chromium based apps.

marisademeglio commented 4 weeks ago

HTML details works well on browsers. Both NVDA and JAWS support it. But I have not tried it on chromium based apps.

We use the details element throughout the UI and I have not heard from users encountering accessibility issues issues with it.

marisademeglio commented 4 weeks ago

@bertfrees @marisademeglio short and detailed messages are now added with a details element and the short message is marked as summary.

It creates an expandable section to display the long message in my tests, the only thing i'm not sure 100% is the accessibility of this element.

For me, there is a 1 second (ish) delay. If I enter a bad API key and press "Connect", it tries to connect and when it fails, it says "Could not connect to the engine, please check your credentials or the service status." just as text, not as a details element. And then after a second, I see "Could not connect to the engine" as a details element with an expandable section with more information.

avneeshsingh commented 4 weeks ago

“HTML details works well on browsers. Both NVDA and JAWS support it. But I have not tried it on chromium based apps.”

OK, if these expandable buttons are HTML details under the hood. Then it works well.

marisademeglio commented 4 weeks ago

Ok I think the implementation solves the issue. Closing.