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

Bug report: chili closes immediately after providing a response #5987

Open
waldekmastykarz opened this issue Apr 20, 2024 · 16 comments · May be fixed by #6025
Open

Bug report: chili closes immediately after providing a response #5987

waldekmastykarz opened this issue Apr 20, 2024 · 16 comments · May be fixed by #6025

Comments

@waldekmastykarz
Copy link
Member

Priority

(Low) Something is a little off

Description

Chili closes immediately after providing a response. It doesn't wait for the user to answer prompts, but instead closes.

Steps to reproduce

m365? "how can I upgrade my SPFx project?"

Expected results

Chili provides a response, waits for the user to answer prompts, and continues as instructed.

Actual results

Chili prints the output, the prompts and closes immediately

Diagnostics

No response

CLI for Microsoft 365 version

Latest from main

nodejs version

v20.11.0

Operating system (environment)

macOS

Shell

zsh

cli doctor

No response

Additional Info

No response

@MathijsVerbeeck
Copy link
Contributor

MathijsVerbeeck commented Apr 20, 2024

I think that it's the same issue as with the previous prompt issue, as we use the ora spinner there as well and we don't add any options when creating the spinner, so this should be an easy fix.

@MathijsVerbeeck
Copy link
Contributor

@waldekmastykarz Something that I noticed too when using the chili command is that the ora spinner also blocks the capability for all the prompts (after I added the discardstdin). So this is another thing that we'd have to research when fixing this. For example; when being prompted for the rating on macOS, it will only show the first two options, and the spinner will be in front of the third option.

@waldekmastykarz
Copy link
Member Author

@waldekmastykarz Something that I noticed too when using the chili command is that the ora spinner also blocks the capability for all the prompts (after I added the discardstdin). So this is another thing that we'd have to research when fixing this. For example; when being prompted for the rating on macOS, it will only show the first two options, and the spinner will be in front of the third option.

Good catch! Let's dive in and see if we can understand what's changed and how to best fix it

@Adam-it
Copy link
Contributor

Adam-it commented Apr 25, 2024

on win + powershell seems to be working 'ok' 🤔
image

however I noticed a different also wrong behavior that the loader seems to be always present 🤔
https://github.com/pnp/cli-microsoft365/assets/58668583/9500db42-f9ef-42fd-8016-af6b33aa6910

@waldekmastykarz
Copy link
Member Author

however I noticed a different also wrong behavior that the loader seems to be always present 🤔

Let's create a separate issue for this. Good catch

@MathijsVerbeeck
Copy link
Contributor

however I noticed a different also wrong behavior that the loader seems to be always present 🤔

Let's create a separate issue for this. Good catch

That's the issue on why the Spinner is blocking the last prompt too.

@Adam-it
Copy link
Contributor

Adam-it commented Apr 26, 2024

@MathijsVerbeeck yes, that's why I think it's kinda connected
Will create a separate issue for that but I hope there will be a single fix for both things

@MathijsVerbeeck
Copy link
Contributor

MathijsVerbeeck commented May 3, 2024

Ok so two things:

  1. We will have to change the initialization for the ora spinner so that we add discardStdin: false for macOS, I'd suggest that we initialize the ora object at the top of the code, and we just set the text dynamically before showing the spinner, so it will be something like this:
const spinner = ora({ discardStdin: false });
if (showSpinner) {
   spinner.text = 'Sending rating...';
   spinner.show();
}
  1. The request that we are currently executing to call the rating endpoint from mendable is not working. It throws the following error:
image I have checked the call that is currently being executed when we use the Mendable on the site from the CLI, but it seems that it is using a different endpoint, which is `eRateMessage`, the only difference being that the parameter `api_key` is named `anon_key` in this request. On their own documentation however, it still shows as if we should use `api_key`.

https://docs.mendable.ai/mendable-api/rating#body-parameters

Due to this request failing, the spinner will keep showing that we are using when sending the rating request.

@milanholemans
Copy link
Contributor

milanholemans commented May 5, 2024

In our code we are using the v0 endpoint, while Mendable docs state to use V1 endpoint. Does this make any difference? If not, I can reach out to the Mendable guys to ask which is the best approach here. We can always copy the request from the Mendable button on the site, but if it's not documented, I don't know if we can just use it.

@Adam-it
Copy link
Contributor

Adam-it commented May 5, 2024

Good catch 😲💪. I think double checking with them over Discord is totally fine 👍

@MathijsVerbeeck
Copy link
Contributor

In our code we are using the 10 endpoint, while Mendable docs state to use V1 endpoint. Does this make any difference? If not, I can reach out to the Mendable guys to ask which is the best approach here. We can always copy the request from the Mendable button on the site, but if it's not documented, I don't know if we can just use it.

it does not make a difference it seems

@milanholemans
Copy link
Contributor

@nickscamara could you guide us in the right direction for this issue, please?

TL;DR: the request we're sending to rate responses is returning a 400 result. Same goes for the request listed in your docs.

@milanholemans
Copy link
Contributor

milanholemans commented May 5, 2024

@MathijsVerbeeck, I reached out to Mendable, they say we should use a server-key instead of an anon-key. Could you try this key, please?

@MathijsVerbeeck
Copy link
Contributor

Can confirm that it works.

@milanholemans
Copy link
Contributor

@pnp/cli-for-microsoft-365-maintainers Nick from Mendable told me that the server-key I gave Mathijs is more sensitive than the anon-key and we shouldn't expose it. I don't know if there is a way for us to not expose it in Chili...
Either we should:

  • Drop rating messages
  • Copy the request from the Mendable component on our docs site, which uses an anon-key if I'm not mistaken. Downside: that request is not documented.

@waldekmastykarz
Copy link
Member Author

I suggest we simplify and drop rating

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

Successfully merging a pull request may close this issue.

4 participants