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

[python] Don't throw error directly when emitter crash #6104

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

msyyc
Copy link
Contributor

@msyyc msyyc commented Feb 21, 2025

fix #6081

We shall not throw error when emitter crash. Instead, we should catch the errors then report them with reportDiagnostic provided by compiler:
image

@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:python Issue for the Python client emitter: @typespec/http-client-python label Feb 21, 2025
@azure-sdk
Copy link
Collaborator

azure-sdk commented Feb 21, 2025

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

@msyyc msyyc marked this pull request as ready for review February 24, 2025 03:32
@microsoft microsoft deleted a comment from azure-sdk Feb 24, 2025
@azure-sdk
Copy link
Collaborator

azure-sdk commented Feb 24, 2025

All changed packages have been documented.

  • @typespec/http-client-python
Show changes

@typespec/http-client-python - fix ✏️

[python] Don't throw error directly when emitter crash

severity: "error",
messages: {
default:
"Python is not installed. Please follow https://www.python.org/ to install Python or set 'use-pyodide' to true.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be an error, because we should automatically call use-pyodide if that is the case. The only way an error like this would occur is if someone explicitly set use-pyodide=false without having python. In that case, I think the error name would be better being something like pyodide-flag-conflict or something like that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense and I think pyodide-flag-conflict is reasonable here.
Updated.

@msyyc msyyc requested a review from iscai-msft February 26, 2025 02:37
"unknown-error": {
severity: "error",
messages: {
default: paramMessage`Can't generate Python client code from this TypeSpec, please open an issue on https://github.com/microsoft/typespec'.\n${"stack"}`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
default: paramMessage`Can't generate Python client code from this TypeSpec, please open an issue on https://github.com/microsoft/typespec'.\n${"stack"}`,
default: paramMessage`Can't generate Python client code from this TypeSpec. Please open an issue on https://github.com/microsoft/typespec'.${"stack"}`,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emitter:client:python Issue for the Python client emitter: @typespec/http-client-python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[python] Emitters should not crash on purpose
4 participants