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

Run App: app may be opened in Viewer before it's ready #5187

Closed
sharon-wang opened this issue Oct 28, 2024 · 3 comments
Closed

Run App: app may be opened in Viewer before it's ready #5187

sharon-wang opened this issue Oct 28, 2024 · 3 comments
Assignees
Labels
area: run app area: workbench Issues related to Workbench category. bug Something isn't working theme: app builder usability

Comments

@sharon-wang
Copy link
Member

System details:

Positron and OS details:

Ubuntu 24.04 x86_64

Positron on Workbench; Latest on main once #5138 is merged.

Browser: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/127.0.0.0 Safari/537.36 Edg/127.0.0.0

Interpreter details:

Python 3.12.3

Describe the issue:

Discovered in #5138. The app url may be opened in the Viewer slightly before the app is actually ready, resulting in an error in the Viewer pane.

Screencast.from.2024-10-28.16-53-29.webm

Steps to reproduce the issue:

Note: this seems to only reproduce in Positron on Workbench.

  1. Open fastapi_example.py
  2. Click the Run App play button
  3. The Viewer opens with the app url, but does not display the app
  4. Click the Viewer refresh button
  5. The app is now visible in the Viewer

Expected or desired behavior:

The app should be ready to interact with in the Viewer after running the app.

Perhaps we can add something like appReadyText to RunAppOptions/DebugAppOptions in https://github.com/posit-dev/positron/blob/main/extensions/positron-run-app/src/positron-run-app.d.ts, which we can then wait for somewhere around here, before we extract the app url to display in the Viewer:

https://github.com/posit-dev/positron/blob/main/extensions/positron-run-app/src/api.ts#L331-L349

In the case of fastapi, we could have appReadyText = Application startup complete.

Were there any error messages in the UI, Output panel, or Developer Tools console?

No

@sharon-wang sharon-wang added bug Something isn't working theme: app builder usability area: workbench Issues related to Workbench category. labels Oct 28, 2024
@sharon-wang sharon-wang self-assigned this Nov 4, 2024
@sharon-wang sharon-wang added this to the 2024.12.0 Pre-Release milestone Nov 4, 2024
sharon-wang added a commit that referenced this issue Nov 6, 2024
- addresses #5187
- ❓ possibly also addresses
#4949

### Implementation Notes
- adds optional `appReadyMessage` property to `RunAppOptions` and
`DebugAppOptions`
- specify an `appReadyMessage` of `'Application startup complete'` for
FastAPI and Shiny apps, which leverage uvicorn and output that message
when the app is ready
- delete empty `DebugConfiguration` object from the
`positron-run-app.d.ts` files, as I think they are unused?
- modified the logging level in `api.ts` for a few messages
- `previewUrlInExecutionOutput` now checks for both the app url and the
`appReadyMessage` if provided,
    - an app is considered ready if:
- the `appReadyMessage` was not provided and the app url has been
matched; or
- the `appReadyMessage` was provided and matched, and the app url has
been matched
- if the `appReadyMessage` was provided but not matched, and the app url
has been matched, we'll time out of the `raceTimeout`, but the url will
have been matched, so we'll still try to open the app in the Viewer

### QA Notes

- I've renabled `Python - Verify Basic FastAPI App [C903306]` which now
passes
- Please use the repro steps from the issue:
#5187
@sharon-wang
Copy link
Member Author

sharon-wang added a commit that referenced this issue Nov 6, 2024
- addresses #5187
- ❓ possibly also addresses
#4949

### Implementation Notes
- adds optional `appReadyMessage` property to `RunAppOptions` and
`DebugAppOptions`
- specify an `appReadyMessage` of `'Application startup complete'` for
FastAPI and Shiny apps, which leverage uvicorn and output that message
when the app is ready
- delete empty `DebugConfiguration` object from the
`positron-run-app.d.ts` files, as I think they are unused?
- modified the logging level in `api.ts` for a few messages
- `previewUrlInExecutionOutput` now checks for both the app url and the
`appReadyMessage` if provided,
    - an app is considered ready if:
- the `appReadyMessage` was not provided and the app url has been
matched; or
- the `appReadyMessage` was provided and matched, and the app url has
been matched
- if the `appReadyMessage` was provided but not matched, and the app url
has been matched, we'll time out of the `raceTimeout`, but the url will
have been matched, so we'll still try to open the app in the Viewer

### QA Notes

- I've renabled `Python - Verify Basic FastAPI App [C903306]` which now
passes
- Please use the repro steps from the issue:
#5187
@sharon-wang
Copy link
Member Author

This fix will be backported to the 2024.11.1 patch in #5284.

@jonvanausdeln
Copy link
Contributor

Verified Fixed

Positron Version(s) : 2024.12.0-15
Workbench Version(s): 2024.11.0+338.pro1
OS Version(s) : Ubuntu 24

Test scenario(s)

Fastapi app loads everytime now as expected.

Link(s) to TestRail test cases run or created:

sharon-wang added a commit that referenced this issue Nov 8, 2024
- Addresses #5281 to bring the fix for #5187 to the patch branch
- Backports the commit from #5265 to the prerelease/2024.11 branch for
the 2024.11.1 pre-release patch
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: run app area: workbench Issues related to Workbench category. bug Something isn't working theme: app builder usability
Projects
None yet
Development

No branches or pull requests

2 participants