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

Add support for parsing error reporting from BufferedSource without filesystem access #6420

Open
samtipton opened this issue Mar 12, 2025 · 7 comments

Comments

@samtipton
Copy link

Use case

I'd like to be able to parse a GQLDocument from a BufferedSource from an in-memory MultipartFile like so

public GQLDocument parseFromMultipartFile(final MultipartFile schemaFile) {
            final BufferedSource buffer =
                    Okio.buffer(Okio.source(new ByteArrayInputStream(schemaFile.getBytes())));
            final GQLDocument gqlDocument = ApolloParser.parseAsGQLDocument(
                            buffer,
                            schemaFile.getOriginalFilename(),
                            ParserOptions.Companion.getDefault())
                    .getOrThrow();
            return gqlDocument;
}

This works if the file is valid graphql SDL. However, if a SourceAwareException is thrown, the file is handled as if it exists on the filesystem and throws a FileNotFoundException

In SourceAwareException:

    private fun preview(error: String, sourceLocation: SourceLocation?): String {
      val preview = if (sourceLocation?.filePath != null && sourceLocation.line >= 1 && sourceLocation.column >= 1) {
        val filePath = sourceLocation.filePath
        val document = try {
          HOST_FILESYSTEM.source(filePath.toPath()).buffer().readUtf8()
        } catch (e: IOException) {
          throw RuntimeException("Failed to read GraphQL file `$this`", e)
        }
       ...

Describe the solution you'd like

No response

@martinbonnin
Copy link
Contributor

Hi 👋

You can pass null for filePath and skip the preview:

            final GQLDocument gqlDocument = ApolloParser.parseAsGQLDocument(
                            buffer,
                            null, // Pass null here
                            ParserOptions.Companion.getDefault())

You will still get the error and line/column where it happens, just not the preview.

Supporting previews for arbitrary sources is quite complicated because some sources cannot rewind and by the time we want to display the preview, the context is consumed already.

@samtipton
Copy link
Author

Hello 👋 !

Ah I see, the buffer is already consumed at this point.
Holding the buffer contents in SourceLocation would be too much?

Just quickly thinking about it I have this

class SourceLocation(
    val start: Int,
    val end: Int,
    val line: Int,
    val column: Int,
    val filePath: String?,
    val rawSource: String? = null
) {..}

Then SourceAwareException would have access to it. This code could definitely be improved.

    private fun preview(error: String, sourceLocation: SourceLocation?): String {
      val preview = if (sourceLocation?.filePath != null && sourceLocation.line >= 1 && sourceLocation.column >= 1) {
        val filePath = sourceLocation.filePath

        val document = try {
          if (!HOST_FILESYSTEM.exists(filePath.toPath()) && sourceLocation.rawSource != null) {
            sourceLocation.rawSource
          }
          else {
            HOST_FILESYSTEM.source(filePath.toPath()).buffer().readUtf8()
          }
        } catch (e: IOException) {
          throw RuntimeException("Failed to read GraphQL file `$this`", e)
        }

@samtipton
Copy link
Author

samtipton commented Mar 12, 2025

Being passed in the extension function

private fun <T : Any> BufferedSource.parseInternal(filePath: String?, options: ParserOptions, block: Parser.() -> T): GQLResult<T> {
  return this.use { source ->
    source.readUtf8().let { content ->
      content.parseInternal(filePath, options, content, block)
    }
  }}

private fun <T : Any> String.parseInternal(filePath: String?, options: ParserOptions, content: String? = null, block: Parser.() -> T): GQLResult<T> {
  return try {
    GQLResult(Parser(this, options, filePath).block(), emptyList())
  } catch (e: ParserException) {
    GQLResult(
        null,
        listOf(
            ParsingError(
                e.message,
                SourceLocation(
                    start = e.token.start,
                    end = e.token.end,
                    line = e.token.line,
                    column = e.token.column,
                    filePath = filePath,
                    rawSource = content
                )
            )
        )
    )
// etc

@martinbonnin
Copy link
Contributor

Holding the buffer contents in SourceLocation would be too much?

We technically probably could with the current parser because it transforms everything in a String but this would lock down the API substantially if we ever wanted to implement a streaming parser one day. Think huge files that holding in memory might not be possible. This is probably never going to be an issue for operations but for schemas maybe?

Note that if you really need this and you have the full contents in a String handy, it should be possible to implement a wrapper function that adds the preview as needed.

@samtipton
Copy link
Author

Hmm, I need to think about how the wrapper would work. Or, its possible to re-buffer the string in source location I suppose. The main issue for me is that SourceAwareException assumes that all sources exist on the file system whereas buffers can be created in memory exclusively.

@martinbonnin
Copy link
Contributor

martinbonnin commented Mar 12, 2025

Something like this should do it:

    public void runTest() throws Exception {
        // Here the input is a String literal but in your example, you could buffer your whole MultiPartFile
        String input = "# a document with a syntax error\n" +
                "type Query {\n" +
                "foo # missing type here\n" +
                "}\n" +
                "# end of the document";
        GQLResult<GQLDocument> result = ApolloParser.parseAsGQLDocument(
                input,
                ParserOptions.Companion.getDefault()
        );
        final GQLDocument gqlDocument = getOrThrow(result, input);
    }

    private GQLDocument getOrThrow(GQLResult<GQLDocument> result, String input) throws Exception {
        if (result.getIssues().isEmpty()) {
            return result.getValue();
        }

        Issue issue = result.getIssues().getFirst();

        List<String> lines = input.lines().toList();

        StringBuilder sb = new StringBuilder();

        int line = issue.getSourceLocation().getLine();
        int column = issue.getSourceLocation().getColumn();
        sb.append(String.format("e: (%d, %d): %s\n", line, column, issue.getMessage()));
        sb.append("------------------------------\n");
        if (line - 2 >= 0) {
            sb.append(String.format("[%d]: %s\n", (line - 1), lines.get(line - 2)));
        }
        sb.append(String.format("[%d]: %s\n", line, lines.get(line - 1)));
        if (line < lines.size()) {
            sb.append(String.format("[%d]: %s\n", (line + 1), lines.get(line)));
        }
        sb.append("------------------------------\n");

        throw new Exception(sb.toString());
    }

@samtipton
Copy link
Author

That is certainly a workaround I am comfortable with. Was coding it up when you posted. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants