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

Update deps #3064

Merged
merged 9 commits into from
May 15, 2023
Merged

Update deps #3064

merged 9 commits into from
May 15, 2023

Conversation

mstoykov
Copy link
Contributor

No description provided.

Seems like the usual amount of random updates for syscalls, bugfixes and
security issues.

The only interesting thing is a fix for http2 connections, where a
failed request will mark the connection as not reusable. Which might
reduce the amount of strange errors (EOF) in some cases, maybe.
@mstoykov mstoykov added this to the v0.45.0 milestone May 11, 2023
@github-actions github-actions bot requested review from imiric and oleiade May 11, 2023 09:26
@mstoykov
Copy link
Contributor Author

grpc is not updated as there are problems - issue pending
prometheus is not updated - to be updated with the extension
browser defependancy also to be updated with browser update

@codecov-commenter
Copy link

codecov-commenter commented May 11, 2023

Codecov Report

Merging #3064 (ac8823e) into master (c00bfbb) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head ac8823e differs from pull request most recent head 29066fb. Consider uploading reports for the commit 29066fb to get more accurate results

@@            Coverage Diff             @@
##           master    #3064      +/-   ##
==========================================
- Coverage   76.72%   76.71%   -0.01%     
==========================================
  Files         229      229              
  Lines       17110    17110              
==========================================
- Hits        13127    13126       -1     
  Misses       3140     3140              
- Partials      843      844       +1     
Flag Coverage Δ
ubuntu 76.71% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 4 files with indirect coverage changes

Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

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

Seems safe for the most part 🤞

Comment on lines +9 to +18
func init() {
// Opt-in for ansi color support for current process.
// https://learn.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences#output-sequences
var outMode uint32
out := windows.Handle(os.Stdout.Fd())
if err := windows.GetConsoleMode(out, &outMode); err != nil {
return
}
outMode |= windows.ENABLE_PROCESSED_OUTPUT | windows.ENABLE_VIRTUAL_TERMINAL_PROCESSING
_ = windows.SetConsoleMode(out, outMode)
Copy link
Contributor

Choose a reason for hiding this comment

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

According to that MS link, this does much more than add color support to Windows terminals. And I think we already supported color on Windows, no? There was some wonky behavior, mainly with Unicode characters, but color was never an issue, AFAIR.

Even if this seems like a safe change, we should still test the behavior on Windows to ensure our UI hacks continue to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even if this seems like a safe change, we should still test the behavior on Windows to ensure our UI hacks continue to work.

I did test both powershell and cmd and it looks exactly the same as before 🤷 .

cmd still does not show unicode and powershell seems as okay as I can expect.

The only discrepancy is that with both old and new version hitting enter while a test is running - does nothing :(.

But that seems like completely unrelated issue and also not really a problem as we do not care about anytnig but ^C 🤞

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From fatih/color#169 (comment) - it is apparently about other terminals that do the more ... accurate handling of colors - where they are not supported unless the application tells them they want them. Which apparently is what this code does.

I unfortunately do not have a terminal that has his problem :(.

Comment on lines +95 to +105
## Catastrophic Backtracking and Timeouts

`regexp2` supports features that can lead to catastrophic backtracking.
`Regexp.MatchTimeout` can be set to to limit the impact of such behavior; the
match will fail with an error after approximately MatchTimeout. No timeout
checks are done by default.

Timeout checking is not free. The current timeout checking implementation starts
a background worker that updates a clock value approximately once every 100
milliseconds. The matching code compares this value against the precomputed
deadline for the match. The performance impact is as follows.
Copy link
Contributor

@imiric imiric May 11, 2023

Choose a reason for hiding this comment

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

This is an indirect dependency used by goja, and MatchTimeout is not used there yet. I guess this is up to Dmitry to decide whether it makes sense, but it could have an impact for us if he decides to enable it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - it is, but goja does not update it as regularly as I would prefer and we already have updated in the past to get some unlikely fixes.

This current update does not matter to use but removes the need to review this change in the future.

Comment on lines +1273 to +1285
if cs.ID != 0 {
// This request may have failed because of a problem with the connection,
// or for some unrelated reason. (For example, the user might have canceled
// the request without waiting for a response.) Mark the connection as
// not reusable, since trying to reuse a dead connection is worse than
// unnecessarily creating a new one.
//
// If cs.ID is 0, then the request was never allocated a stream ID and
// whatever went wrong was unrelated to the connection. We might have
// timed out waiting for a stream slot when StrictMaxConcurrentStreams
// is set, for example, in which case retrying on a different connection
// will not help.
cs.cc.doNotReuse = true
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a good thing, and probably safe for us(?) 🤞

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am hopeful that this will fix some EOF errors for some users. Also this is very likely to already have landed in stdlib as it is just x/net vendored.

github.com/go-sourcemap/sourcemap v2.1.4-0.20211119122758-180fcef48034+incompatible
github.com/golang/protobuf v1.5.2
github.com/gorilla/websocket v1.5.0
github.com/grafana/xk6-browser v0.9.0
github.com/grafana/xk6-output-prometheus-remote v0.1.0
github.com/grafana/xk6-output-prometheus-remote v0.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

@mstoykov mstoykov merged commit f18bd84 into master May 15, 2023
@mstoykov mstoykov deleted the updateDeps branch May 15, 2023 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants