76c95de · crush · git.iko.soy

git.iko.soy/crush

on main help sign in

fix: don’t panic when fetching document URI path fails

76c95deChristian Rocha1 year ago

internal/llm/tools/diagnostics.go+15−3

compare with main

unifiedsplit

@@ -4,6 +4,7 @@ import (

4 "context"

5 "encoding/json"

6 "fmt"

7+ "log/slog"

7 "maps"

8 "maps"

8 "sort"

9 "sort"

9 "strings"

10 "strings"

@@ -118,7 +119,13 @@ func waitForLspDiagnostics(ctx context.Context, filePath string, lsps map[string

118 return

119 return

119 }

120 }

120

121

122+ path, err := diagParams.URI.Path()

123+ if err != nil {

124+ slog.Error("Failed to convert diagnostic URI to path", "uri", diagParams.URI, "error", err)

125+ return

126+ }

127+

121− if diagParams.URI.Path() == filePath || hasDiagnosticsChanged(client.GetDiagnostics(), originalDiags) {

128+ if path == filePath || hasDiagnosticsChanged(client.GetDiagnostics(), originalDiags) {

122 select {

129 select {

123 case diagChan <- struct{}{}:

130 case diagChan <- struct{}{}:

124 default:

131 default:

@@ -216,10 +223,15 @@ func getDiagnostics(filePath string, lsps map[string]*lsp.Client) string {

216 diagnostics := client.GetDiagnostics()

223 diagnostics := client.GetDiagnostics()

217 if len(diagnostics) > 0 {

224 if len(diagnostics) > 0 {

218 for location, diags := range diagnostics {

225 for location, diags := range diagnostics {

226+ path, err := location.Path()

227+ if err != nil {

228+ slog.Error("Failed to convert diagnostic location URI to path", "uri", location, "error", err)

229+ continue

230+ }

219− isCurrentFile := location.Path() == filePath

231+ isCurrentFile := path == filePath

220

232

221 for _, diag := range diags {

233 for _, diag := range diags {

222− formattedDiag := formatDiagnostic(location.Path(), diag, lspName)

234+ formattedDiag := formatDiagnostic(path, diag, lspName)

223

235

224 if isCurrentFile {

236 if isCurrentFile {

225 fileDiagnostics = append(fileDiagnostics, formattedDiag)

237 fileDiagnostics = append(fileDiagnostics, formattedDiag)

internal/lsp/client.go+11−2

@@ -449,7 +449,12 @@ func (c *Client) pingTypeScriptServer(ctx context.Context) error {

449

450 // If we have any open files, try to get document symbols for one

451 for uri := range c.openFiles {

452− filePath := protocol.DocumentURI(uri).Path()

452+ filePath, err := protocol.DocumentURI(uri).Path()

453+ if err != nil {

454+ slog.Error("Failed to convert URI to path for TypeScript symbol collection", "uri", uri, "error", err)

455+ continue

456+ }

457+

453 if strings.HasSuffix(filePath, ".ts") || strings.HasSuffix(filePath, ".js") ||

458 if strings.HasSuffix(filePath, ".ts") || strings.HasSuffix(filePath, ".js") ||

454 strings.HasSuffix(filePath, ".tsx") || strings.HasSuffix(filePath, ".jsx") {

459 strings.HasSuffix(filePath, ".tsx") || strings.HasSuffix(filePath, ".jsx") {

455 var symbols []protocol.DocumentSymbol

460 var symbols []protocol.DocumentSymbol

@@ -712,7 +717,11 @@ func (c *Client) CloseAllFiles(ctx context.Context) {

712 // First collect all URIs that need to be closed

717 // First collect all URIs that need to be closed

713 for uri := range c.openFiles {

718 for uri := range c.openFiles {

714 // Convert URI back to file path using proper URI handling

719 // Convert URI back to file path using proper URI handling

715− filePath := protocol.DocumentURI(uri).Path()

720+ filePath, err := protocol.DocumentURI(uri).Path()

721+ if err != nil {

722+ slog.Error("Failed to convert URI to path for file closing", "uri", uri, "error", err)

723+ continue

724+ }

716 filesToClose = append(filesToClose, filePath)

725 filesToClose = append(filesToClose, filePath)

717 }

726 }

718 c.openFilesMu.Unlock()

727 c.openFilesMu.Unlock()

internal/lsp/protocol/pattern_interfaces.go+18−2

@@ -2,6 +2,7 @@ package protocol

3import (

4 "fmt"

5+ "log/slog"

7// PatternInfo is an interface for types that represent glob patterns

8// PatternInfo is an interface for types that represent glob patterns

@@ -36,21 +37,36 @@ func (g *GlobPattern) AsPattern() (PatternInfo, error) {

36 return nil, fmt.Errorf("nil pattern")

37 return nil, fmt.Errorf("nil pattern")

37 }

38 }

40+ var err error

41+

39 switch v := g.Value.(type) {

42 switch v := g.Value.(type) {

40 case string:

43 case string:

41 return StringPattern{Pattern: v}, nil

44 return StringPattern{Pattern: v}, nil

45+

42 case RelativePattern:

46 case RelativePattern:

43 // Handle BaseURI which could be string or DocumentUri

47 // Handle BaseURI which could be string or DocumentUri

44 basePath := ""

48 basePath := ""

45 switch baseURI := v.BaseURI.Value.(type) {

49 switch baseURI := v.BaseURI.Value.(type) {

46 case string:

50 case string:

47− basePath = DocumentURI(baseURI).Path()

51+ basePath, err = DocumentURI(baseURI).Path()

52+ if err != nil {

53+ slog.Error("Failed to convert URI to path", "uri", baseURI, "error", err)

54+ return nil, fmt.Errorf("invalid URI: %s", baseURI)

55+ }

56+

48 case DocumentURI:

57 case DocumentURI:

49− basePath = baseURI.Path()

58+ basePath, err = baseURI.Path()

59+ if err != nil {

60+ slog.Error("Failed to convert DocumentURI to path", "uri", baseURI, "error", err)

61+ return nil, fmt.Errorf("invalid DocumentURI: %s", baseURI)

62+ }

63+

50 default:

64 default:

51 return nil, fmt.Errorf("unknown BaseURI type: %T", v.BaseURI.Value)

65 return nil, fmt.Errorf("unknown BaseURI type: %T", v.BaseURI.Value)

52 }

66 }

67+

53 return RelativePatternInfo{RP: v, BasePath: basePath}, nil

68 return RelativePatternInfo{RP: v, BasePath: basePath}, nil

69+

54 default:

70 default:

55 return nil, fmt.Errorf("unknown pattern type: %T", g.Value)

71 return nil, fmt.Errorf("unknown pattern type: %T", g.Value)

56 }

72 }

internal/lsp/protocol/uri.go+18−7

@@ -70,7 +70,7 @@ func (uri *DocumentURI) UnmarshalText(data []byte) (err error) {

70// DocumentUri("").Path() returns the empty string.

71//

72// Path panics if called on a URI that is not a valid filename.

73−func (uri DocumentURI) Path() string {

73+func (uri DocumentURI) Path() (string, error) {

74 filename, err := filename(uri)

75 if err != nil {

76 // e.g. ParseRequestURI failed.

@@ -79,22 +79,33 @@ func (uri DocumentURI) Path() string {

79 // direct string manipulation; all DocumentUris

80 // received from the client pass through

81 // ParseRequestURI, which ensures validity.

82− panic(err)

82+ return "", fmt.Errorf("invalid URI %q: %w", uri, err)

83 }

84− return filepath.FromSlash(filename)

84+ return filepath.FromSlash(filename), nil

85}

87// Dir returns the URI for the directory containing the receiver.

88−func (uri DocumentURI) Dir() DocumentURI {

88+func (uri DocumentURI) Dir() (DocumentURI, error) {

89+ // XXX: Legacy comment:

89 // This function could be more efficiently implemented by avoiding any call

90 // This function could be more efficiently implemented by avoiding any call

90 // to Path(), but at least consolidates URI manipulation.

91 // to Path(), but at least consolidates URI manipulation.

92+

93+ path, err := uri.DirPath()

94+ if err != nil {

95+ return "", fmt.Errorf("invalid URI %q: %w", uri, err)

96+ }

97+

91− return URIFromPath(uri.DirPath())

98+ return URIFromPath(path), nil

92}

99}

100

94// DirPath returns the file path to the directory containing this URI, which

101// DirPath returns the file path to the directory containing this URI, which

95// must be a file URI.

102// must be a file URI.

96−func (uri DocumentURI) DirPath() string {

103+func (uri DocumentURI) DirPath() (string, error) {

104+ path, err := uri.Path()

105+ if err != nil {

106+ return "", err

107+ }

97− return filepath.Dir(uri.Path())

108+ return filepath.Dir(path), nil

98}

109}

110

100func filename(uri DocumentURI) (string, error) {

111func filename(uri DocumentURI) (string, error) {

internal/lsp/util/edit.go+27−5

@@ -11,7 +11,10 @@ import (

11)

13func applyTextEdits(uri protocol.DocumentURI, edits []protocol.TextEdit) error {

14− path := uri.Path()

14+ path, err := uri.Path()

15+ if err != nil {

16+ return fmt.Errorf("invalid URI: %w", err)

17+ }

16 // Read the file content

19 // Read the file content

17 content, err := os.ReadFile(path)

20 content, err := os.ReadFile(path)

@@ -148,7 +151,11 @@ func applyTextEdit(lines []string, edit protocol.TextEdit) ([]string, error) {

148// applyDocumentChange applies a DocumentChange (create/rename/delete operations)

151// applyDocumentChange applies a DocumentChange (create/rename/delete operations)

149func applyDocumentChange(change protocol.DocumentChange) error {

152func applyDocumentChange(change protocol.DocumentChange) error {

150 if change.CreateFile != nil {

153 if change.CreateFile != nil {

151− path := change.CreateFile.URI.Path()

154+ path, err := change.CreateFile.URI.Path()

155+ if err != nil {

156+ return fmt.Errorf("invalid URI: %w", err)

157+ }

158+

152 if change.CreateFile.Options != nil {

159 if change.CreateFile.Options != nil {

153 if change.CreateFile.Options.Overwrite {

160 if change.CreateFile.Options.Overwrite {

154 // Proceed with overwrite

161 // Proceed with overwrite

@@ -164,7 +171,11 @@ func applyDocumentChange(change protocol.DocumentChange) error {

164 }

171 }

165

172

166 if change.DeleteFile != nil {

173 if change.DeleteFile != nil {

167− path := change.DeleteFile.URI.Path()

174+ path, err := change.DeleteFile.URI.Path()

175+ if err != nil {

176+ return fmt.Errorf("invalid URI: %w", err)

177+ }

178+

168 if change.DeleteFile.Options != nil && change.DeleteFile.Options.Recursive {

179 if change.DeleteFile.Options != nil && change.DeleteFile.Options.Recursive {

169 if err := os.RemoveAll(path); err != nil {

180 if err := os.RemoveAll(path); err != nil {

170 return fmt.Errorf("failed to delete directory recursively: %w", err)

181 return fmt.Errorf("failed to delete directory recursively: %w", err)

@@ -177,8 +188,19 @@ func applyDocumentChange(change protocol.DocumentChange) error {

177 }

188 }

178

189

179 if change.RenameFile != nil {

190 if change.RenameFile != nil {

191+ var newPath, oldPath string

192+ var err error

193+

180− oldPath := change.RenameFile.OldURI.Path()

194+ oldPath, err = change.RenameFile.OldURI.Path()

195+ if err != nil {

196+ return err

197+ }

198+

181− newPath := change.RenameFile.NewURI.Path()

199+ newPath, err = change.RenameFile.NewURI.Path()

200+ if err != nil {

201+ return err

202+ }

203+

182 if change.RenameFile.Options != nil {

204 if change.RenameFile.Options != nil {

183 if !change.RenameFile.Options.Overwrite {

205 if !change.RenameFile.Options.Overwrite {

184 if _, err := os.Stat(newPath); err == nil {

206 if _, err := os.Stat(newPath); err == nil {

internal/lsp/watcher/watcher.go+12−2

@@ -617,7 +617,11 @@ func (w *WorkspaceWatcher) matchesPattern(path string, pattern protocol.GlobPatt

617 return false

618 }

619 // For relative patterns

620− basePath = protocol.DocumentURI(basePath).Path()

620+ if basePath, err = protocol.DocumentURI(basePath).Path(); err != nil {

621+ // XXX: Do we want to return here, or send the error up the stack?

622+ slog.Error("Error converting base path to URI", "basePath", basePath, "error", err)

623+ }

624+

621 basePath = filepath.ToSlash(basePath)

625 basePath = filepath.ToSlash(basePath)

622

626

623 // Make path relative to basePath for matching

627 // Make path relative to basePath for matching

@@ -660,7 +664,13 @@ func (w *WorkspaceWatcher) debounceHandleFileEvent(ctx context.Context, uri stri

660// handleFileEvent sends file change notifications

664// handleFileEvent sends file change notifications

661func (w *WorkspaceWatcher) handleFileEvent(ctx context.Context, uri string, changeType protocol.FileChangeType) {

665func (w *WorkspaceWatcher) handleFileEvent(ctx context.Context, uri string, changeType protocol.FileChangeType) {

662 // If the file is open and it's a change event, use didChange notification

666 // If the file is open and it's a change event, use didChange notification

663− filePath := protocol.DocumentURI(uri).Path()

667+ filePath, err := protocol.DocumentURI(uri).Path()

668+ if err != nil {

669+ // XXX: Do we want to return here, or send the error up the stack?

670+ slog.Error("Error converting URI to path", "uri", uri, "error", err)

671+ return

672+ }

673+

664 if changeType == protocol.FileChangeType(protocol.Deleted) {

674 if changeType == protocol.FileChangeType(protocol.Deleted) {

665 w.client.ClearDiagnosticsForURI(protocol.DocumentURI(uri))

675 w.client.ClearDiagnosticsForURI(protocol.DocumentURI(uri))

666 } else if changeType == protocol.FileChangeType(protocol.Changed) && w.client.IsFileOpen(filePath) {

676 } else if changeType == protocol.FileChangeType(protocol.Changed) && w.client.IsFileOpen(filePath) {