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

Repeated output with pauses + newlines loses text #4519

Closed
hadley opened this issue Aug 28, 2024 · 7 comments
Closed

Repeated output with pauses + newlines loses text #4519

hadley opened this issue Aug 28, 2024 · 7 comments
Assignees
Labels
area: console Issues related to Console category. bug Something isn't working lang: r

Comments

@hadley
Copy link

hadley commented Aug 28, 2024

System details:

Positron and OS details:

Positron Version: 2024.07.0 (Universal) build 125
Code - OSS Version: 1.91.0
Commit: cae4905
Date: 2024-08-01T00:27:06.019Z (3 wks ago)
Electron: 29.4.0
Chromium: 122.0.6261.156
Node.js: 20.9.0
V8: 12.2.281.27-electron.0
OS: Darwin arm64 23.5.0

Interpreter details:

R 4.4.0

Describe the issue:

Sometimes printed output is silently lost from the console.

Steps to reproduce the issue:

tokens <- c("", "Why", " do", " programmers", " prefer", " dark", " mode", 
"?\n\n", "Because", " light", " attracts", " bugs", "!")

# Always ok
for(token in tokens) {cat(token)}

# Sometimes misses on one or words in the second paragraph
for(token in tokens) {cat(token); Sys.sleep(0.01)}

Expected or desired behavior:

Output shouldn't go missing 😄

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

@hadley
Copy link
Author

hadley commented Aug 28, 2024

Just updated positron and I still see the problem:

Positron Version: 2024.08.0 (Universal) build 48
Code - OSS Version: 1.91.0
Commit: ed616b3
Date: 2024-08-19T04:26:51.868Z (1 wk ago)
Electron: 29.4.0
Chromium: 122.0.6261.156
Node.js: 20.9.0
V8: 12.2.281.27-electron.0
OS: Darwin arm64 23.5.0

@juliasilge
Copy link
Contributor

Do we think this may be another example of problems arising from our current approach to #1762? It is a for() loop where things are getting dropped, however, and not literal lines of code.

@DavisVaughan
Copy link
Contributor

This is likely a unique bug we haven't seen before

@juliasilge juliasilge added bug Something isn't working area: console Issues related to Console category. lang: r labels Aug 29, 2024
@juliasilge juliasilge added this to the Future milestone Sep 3, 2024
@hadley
Copy link
Author

hadley commented Sep 24, 2024

I was reminded of this issue again today as I was working on elmer. It makes it really difficult to use streaming output (which you really want to do for a good user experience).

@DavisVaughan
Copy link
Contributor

DavisVaughan commented Sep 24, 2024

Bumping priority to 2024.11

@DavisVaughan
Copy link
Contributor

DavisVaughan commented Sep 27, 2024

Enabling trace mode shows that I don't think this is an ark issue, I think it is a Console issue. Ark is indeed forwarding all relevant output to the frontend. In this case part of that 2nd stream message is getting dropped somehow

Screenshot 2024-09-27 at 3 23 26 PM

@DavisVaughan DavisVaughan self-assigned this Sep 30, 2024
DavisVaughan added a commit that referenced this issue Sep 30, 2024
…eam specialization (#4848)

Addresses #4519

In `addActivityItemStream()`, either a `ActivityItemOutputStream` or
`ActivityItemErrorStream` always goes in, but it was possible for a
"generic" `ActivityItemStream` to come out here when we make one from
the `remainderText` after the last newline:


https://github.com/posit-dev/positron/blob/e51332d5baa59b72f5e8b28861bbb751f25452ea/src/vs/workbench/services/positronConsole/browser/classes/activityItemStream.ts#L118-L136

That generic `ActivityItemStream` would get set here:

https://github.com/posit-dev/positron/blob/c9504d307c686ce2deefdc6887ad2f7b044dd2e9/src/vs/workbench/services/positronConsole/browser/classes/runtimeItemActivity.ts#L110-L116

And then pushed onto `this._activityItems`

https://github.com/posit-dev/positron/blob/c9504d307c686ce2deefdc6887ad2f7b044dd2e9/src/vs/workbench/services/positronConsole/browser/classes/runtimeItemActivity.ts#L138-L139

The problem is that our Console code doesn't know how to deal with a
generic `ActivityItemStream`. It must be a specialized
`ActivityItemOutputStream` or `ActivityItemErrorStream`!

https://github.com/posit-dev/positron/blob/c9504d307c686ce2deefdc6887ad2f7b044dd2e9/src/vs/workbench/contrib/positronConsole/browser/components/runtimeActivity.tsx#L46-L49

If a generic `ActivityItemStream` slipped through, the output would
never appear in the Console, making it look "dropped", as reported in
#4519.

---

I like how we have a generic `ActivityItemStream` class that we
specialize for the output/error types, so I've fixed this bug by using
[polymorphic
`this`](https://www.typescriptlang.org/docs/handbook/advanced-types.html#polymorphic-this-types)
to allow `addActivityItemStream()` to take and return objects of the
correct specialized type.

Ideally we'd simply be able to use `new this.constructor()` in place of
`new ActivityItemStream()` to use the polymorphic constructor when we
need to create new objects, but due to some [well known
weirdness](microsoft/TypeScript#3841) in
typescript, you have to manually cast it to the constructor type, so
`newActivityItemStream()` wraps that up.

I've also made the `ActivityItemStream` constructor `protected` to avoid
us using it by accident in the codebase. We should _always_ be creating
`ActivityItemOutputStream` or `ActivityItemErrorStream` instead.

Before 😢 



https://github.com/user-attachments/assets/b1854d9e-94eb-4952-aead-bebcf5c5d6a2



After 🥳 


https://github.com/user-attachments/assets/4a76974c-6862-4e84-a296-620717ddb8da
@testlabauto
Copy link
Contributor

Verified Fixed

Positron Version(s) : 2024.09.0-107
OS Version          : OSX

Test scenario(s)

No output is dropped now.

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: console Issues related to Console category. bug Something isn't working lang: r
Projects
None yet
Development

No branches or pull requests

4 participants