Skip to content

fix: Turn any log streaming related errors to warnings#876

Merged
Pijukatel merged 2 commits intomasterfrom
suppres-streamed-log-connection-loss
Mar 26, 2026
Merged

fix: Turn any log streaming related errors to warnings#876
Pijukatel merged 2 commits intomasterfrom
suppres-streamed-log-connection-loss

Conversation

@Pijukatel
Copy link
Copy Markdown
Contributor

Description

Errors in log streaming should not propagate further. Just log them as warnings.
Such errors should be fixed on API level: https://github.com/apify/apify-core/issues/26653

Issues

Closes: #873

@github-actions github-actions bot added this to the 137th sprint - Tooling team milestone Mar 26, 2026
@github-actions github-actions bot added t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics. labels Mar 26, 2026
@Pijukatel Pijukatel requested a review from barjin March 26, 2026 12:20
@Pijukatel Pijukatel force-pushed the suppres-streamed-log-connection-loss branch from 3be348d to 34ba9ca Compare March 26, 2026 12:22
@Pijukatel Pijukatel force-pushed the suppres-streamed-log-connection-loss branch from 34ba9ca to 431c428 Compare March 26, 2026 12:25
Copy link
Copy Markdown
Member

@barjin barjin left a comment

Choose a reason for hiding this comment

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

Nice, I didn't even hope it would be this simple 😄

I have one nit regarding the warning message formatting, otherwise lgtm. Thank you @Pijukatel !

this.destinationLog.info(lastMessage);
}
} catch (err) {
this.destinationLog.warning(`Log redirection stopped due to error: ${JSON.stringify(err)}`);
Copy link
Copy Markdown
Member

@barjin barjin Mar 26, 2026

Choose a reason for hiding this comment

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

There is a second data parameter in all(?) Log methods (code), where you can pass structured data which gets serialized using the logger-wide rules. This might help with the err printing (JSON serialization just makes everything an opaque, one-line string)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. Also, it made me realize that this should be logged by the normal logger and not the redirect one, because this is not the log from the redirected actor run.

@Pijukatel Pijukatel force-pushed the suppres-streamed-log-connection-loss branch from 0617ccf to 0b879f0 Compare March 26, 2026 14:23
@Pijukatel Pijukatel merged commit a3268ee into master Mar 26, 2026
8 of 9 checks passed
@Pijukatel Pijukatel deleted the suppres-streamed-log-connection-loss branch March 26, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

actor.call() with log streaming throws network error on long runs

3 participants