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 https for the API calls or allow setting the protocol #465

Closed
ianjakobs opened this issue Jan 11, 2022 · 6 comments
Closed

Use https for the API calls or allow setting the protocol #465

ianjakobs opened this issue Jan 11, 2022 · 6 comments

Comments

@ianjakobs
Copy link

First reported in #458, myself and a few others are still experiencing the following error despite the original issue having been closed by the author:

Unable to connect: StatusCode: 500, Protocol "http:" not supported. Expected "https:"

More details can be found there, but in short it seems that this SDK doesn't explicitly set a protocol for the API calls, causing the Sentry JavaScript SDK to fall back to http.

Even though it looks like it's only us Sentry users that are running into this, it seems better to address it here since the Azure API requires https. Alternatively, it could help to be able to set the protocol explicitly through sdk.SpeechConfig.fromEndpoint, which is what I've tried to do as well.

For reference, here's the code that I'm using:

const synthesizeSpeech = async (
    term: Translation['term'],
    language: Translation['to'],
): Promise<sdk.SpeechSynthesisResult> => {
    const speechConfig = sdk.SpeechConfig.fromSubscription(
        process.env.MICROSOFT_SPEECH_KEY,
        process.env.MICROSOFT_SPEECH_REGION,
    )

    speechConfig.speechSynthesisLanguage = language
    speechConfig.speechSynthesisVoiceName = LANGUAGES.find(({ code }) => code === language).voice
    speechConfig.speechSynthesisOutputFormat = sdk.SpeechSynthesisOutputFormat.Audio24Khz48KBitRateMonoMp3

    const synthesizer = new sdk.SpeechSynthesizer(speechConfig)

    return new Promise((resolve, reject) => synthesizer.speakTextAsync(
        term,
        result => {
            resolve(result)
            synthesizer.close()
        },
        error => {
            console.error(error)
            reject(error)
            synthesizer.close()
        }),
    )
}
@rhurey
Copy link
Member

rhurey commented Jan 11, 2022

Some debugging this morning...

The tl;dr It looks like this is some friction between how the agent-base module decides if the connection is secure or not (It scans the callstack at the point of the request) and when Sentry is trying to determine what the protocol is.

@rhurey
Copy link
Member

rhurey commented Jan 13, 2022

Looks like TooTallNate/node-agent-base#61 is the most likely root cause.

Have a workaround I'll send a PR for.

@rhurey
Copy link
Member

rhurey commented Jan 13, 2022

@ianjakobs any chance you could give this branch a try? https://github.com/microsoft/cognitive-services-speech-sdk-js/tree/rhurey/add_protocol

I ran a small test w/ sentry and it seems to work now, but...

@ianjakobs
Copy link
Author

Yes, that seems to work! I installed from your branch with npm i microsoft/cognitive-services-speech-sdk-js#rhurey/add_protocol and followed the recommended implementation for Next.js (as I did before) and I'm now getting the expected response.

Thank you for the fast response and fix!

@rhurey
Copy link
Member

rhurey commented Jan 13, 2022

Ok, the PR is in so I'm going to close this issue, lmk if you have problems.
#468

@rhurey rhurey closed this as completed Jan 13, 2022
@dargilco
Copy link
Member

dargilco commented Feb 2, 2022

From the 1.20 release notes:
JavaScript: Add protocol to agent to mitigate bug found with Sentry integration (GitHub issue 465)

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

3 participants