Skip to content

Commit

Permalink
Fix to remove browser downloads path
Browse files Browse the repository at this point in the history
Each BrowserContext has a downloads path. We now clean them up when the
Browser they are associated with closes.
  • Loading branch information
inancgumus committed Jan 28, 2025
1 parent da7db7e commit cf3aa58
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
13 changes: 12 additions & 1 deletion internal/js/modules/k6/browser/common/browser.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ type Browser struct {
// version caches the browser version information.
version browserVersion

// runOnClose is a list of functions to run when the browser is closed.
runOnClose []func() error

logger *log.Logger
}

Expand Down Expand Up @@ -161,6 +164,7 @@ func (b *Browser) connect() error {
if err != nil {
return fmt.Errorf("browser connect: %w", err)
}
b.runOnClose = append(b.runOnClose, b.defaultContext.cleanup)

return b.initEvents()
}
Expand All @@ -172,7 +176,6 @@ func (b *Browser) disposeContext(id cdp.BrowserContextID) error {
if err := action.Do(cdp.WithExecutor(b.vuCtx, b.conn)); err != nil {
return fmt.Errorf("disposing browser context ID %s: %w", id, err)
}

b.context = nil

return nil
Expand Down Expand Up @@ -532,6 +535,13 @@ func (b *Browser) Close() {
b.logger.Errorf("Browser:Close", "cleaning up the user data directory: %v", err)
}
}()
defer func() {
for _, fn := range b.runOnClose {
if err := fn(); err != nil {
b.logger.Errorf("Browser:Close", "running cleanup function: %v", err)
}
}
}()

b.logger.Debugf("Browser:Close", "")
atomic.CompareAndSwapInt64(&b.state, b.state, BrowserStateClosed)
Expand Down Expand Up @@ -629,6 +639,7 @@ func (b *Browser) NewContext(opts *BrowserContextOptions) (*BrowserContext, erro
spanRecordError(span, err)
return nil, err
}
b.runOnClose = append(b.runOnClose, browserCtx.cleanup)

b.contextMu.Lock()
defer b.contextMu.Unlock()
Expand Down
10 changes: 10 additions & 0 deletions internal/js/modules/k6/browser/common/browser_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,16 @@ func (b *BrowserContext) setDownloadsPath(path string) error {
return nil
}

// cleanup cleans up the resources associated with the browser context.
func (b *BrowserContext) cleanup() error {
if err := os.RemoveAll(b.DownloadsPath); err != nil { //nolint:forbidigo
return fmt.Errorf("removing downloads path: %w", err)
}
b.DownloadsPath = ""

return nil
}

// NewBrowserContext creates a new browser context.
func NewBrowserContext(
ctx context.Context, browser *Browser, id cdp.BrowserContextID, opts *BrowserContextOptions, logger *log.Logger,
Expand Down
9 changes: 9 additions & 0 deletions internal/js/modules/k6/browser/common/browser_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ func TestSetDownloadsPath(t *testing.T) {
require.NoError(t, bc.setDownloadsPath(path))
assert.Equal(t, path, bc.DownloadsPath)
})
t.Run("cleanup", func(t *testing.T) {
t.Parallel()

var bc BrowserContext
require.NoError(t, bc.setDownloadsPath(""))
assert.DirExists(t, bc.DownloadsPath)
require.NoError(t, bc.cleanup())
assert.NoDirExists(t, bc.DownloadsPath)
})
}

func TestFilterCookies(t *testing.T) {
Expand Down

0 comments on commit cf3aa58

Please sign in to comment.