diff --git a/src/main/java/org/kohsuke/github/GHGraphQLException.java b/src/main/java/org/kohsuke/github/GHGraphQLException.java new file mode 100644 index 0000000000..b9b9a7f55a --- /dev/null +++ b/src/main/java/org/kohsuke/github/GHGraphQLException.java @@ -0,0 +1,101 @@ +package org.kohsuke.github; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import org.kohsuke.github.internal.graphql.response.GHGraphQLError; + +import java.util.Collections; +import java.util.List; + +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; + +/** + * Thrown when a GitHub GraphQL request returns a successful HTTP response whose body contains a non-empty + * {@code errors} array. + * + *

+ * This exception preserves the structured error list so callers can branch on the error {@link GHGraphQLError#getType() + * type}, {@link GHGraphQLError#getPath() path}, and other fields. Partial response data is exposed through + * {@link #getResponseData()} and the originating query through {@link #getQuery()} to ease debugging. + *

+ * + *

+ * HTTP-level failures (4xx/5xx) do not surface as this exception; they continue to be reported as {@link HttpException} + * or its subclasses, since the response cannot be parsed as a GraphQL payload. + *

+ * + * @see GraphQL spec — Errors + */ +public class GHGraphQLException extends GHIOException { + + private static final long serialVersionUID = 1L; + + /** + * Structured GraphQL errors. Marked transient because {@link GHGraphQLError} carries Jackson-populated + * {@code Object} fields that may not be {@link java.io.Serializable}. + */ + private final transient List errors; + + /** Original GraphQL query, when available; useful for debugging. */ + private final String query; + + /** Partial response data, transient because the payload type is unconstrained. */ + private final transient Object responseData; + + /** + * Instantiates a new GraphQL exception. + * + * @param message + * human-readable summary suitable for log output + * @param errors + * the structured GraphQL errors returned by the server, never {@code null} + * @param responseData + * the partial {@code data} payload, or {@code null} if the server returned none + * @param query + * the GraphQL query string sent in the originating request, or {@code null} if unknown + */ + @SuppressFBWarnings(value = { "EI_EXPOSE_REP2" }, justification = "errors list is wrapped unmodifiable") + public GHGraphQLException(@Nonnull String message, + @Nonnull List errors, + @CheckForNull Object responseData, + @CheckForNull String query) { + super(message); + this.errors = Collections.unmodifiableList(errors); + this.responseData = responseData; + this.query = query; + } + + /** + * Get the structured error entries returned by the GraphQL endpoint. The list is unmodifiable. Returns an empty + * list after this exception has been deserialized across a JVM boundary, since the structured errors are not + * preserved through serialization. + * + * @return the GraphQL errors + */ + @Nonnull + public List getErrors() { + return errors == null ? Collections.emptyList() : errors; + } + + /** + * Get the GraphQL query that produced this response, when available. + * + * @return the query string, or {@code null} + */ + @CheckForNull + public String getQuery() { + return query; + } + + /** + * Get the partial response data returned alongside the errors, if any. GraphQL allows servers to return both + * {@code data} and {@code errors} when a request only partially fails. Returns {@code null} when the server + * returned no data or after deserialization, e.g. across the network. + * + * @return the partial response data, or {@code null} + */ + @CheckForNull + public Object getResponseData() { + return responseData; + } +} diff --git a/src/main/java/org/kohsuke/github/Requester.java b/src/main/java/org/kohsuke/github/Requester.java index 95f0366ebd..67095eca90 100644 --- a/src/main/java/org/kohsuke/github/Requester.java +++ b/src/main/java/org/kohsuke/github/Requester.java @@ -102,6 +102,9 @@ public T fetch(@Nonnull Class type) throws IOException { * @param type * the type * @return an instance of {@code GHGraphQLResponse} + * @throws GHGraphQLException + * if the server returns a successful HTTP response whose body contains a non-empty {@code errors} + * array. * @throws IOException * if the server returns 4xx/5xx responses. */ @@ -109,7 +112,11 @@ public , S> S fetchGraphQL(@Nonnull Class type T response = fetch(type); if (!response.isSuccessful()) { - throw new IOException("GraphQL request failed by:" + response.getErrorMessages()); + String message = response.buildErrorSummary("Request failed due to following response errors"); + throw new GHGraphQLException(message, + response.getErrors(), + response.getDataUnchecked(), + extractGraphQLQuery()); } return response.getData(); @@ -201,4 +208,19 @@ public PagedIterable toIterable(Class type, Consumer itemInitiali return new GitHubPageContentsIterable<>(client, build(), type, itemInitializer); } + + /** + * Best-effort lookup of the {@code query} parameter set by {@link GitHub#createGraphQLRequest(String)}. Returns + * {@code null} if the parameter is missing, e.g. when the request was assembled by hand. + * + * @return the query string sent in this request, or {@code null} + */ + private String extractGraphQLQuery() { + for (GitHubRequest.Entry entry : build().args()) { + if ("query".equals(entry.key) && entry.value instanceof String) { + return (String) entry.value; + } + } + return null; + } } diff --git a/src/main/java/org/kohsuke/github/internal/graphql/response/GHGraphQLError.java b/src/main/java/org/kohsuke/github/internal/graphql/response/GHGraphQLError.java new file mode 100644 index 0000000000..7360addb6c --- /dev/null +++ b/src/main/java/org/kohsuke/github/internal/graphql/response/GHGraphQLError.java @@ -0,0 +1,135 @@ +package org.kohsuke.github.internal.graphql.response; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import javax.annotation.CheckForNull; + +/** + * A single error entry returned by a GitHub GraphQL response. + * + *

+ * Per the GraphQL specification, only {@code message} is guaranteed to be present. Other fields ({@code type}, + * {@code path}, {@code locations}, {@code extensions}) are optional and may be {@code null} depending on the error. + * Unknown fields are ignored to remain forward-compatible with future server-side additions. + *

+ * + * @see GraphQL spec — Errors + * @see GitHub GraphQL — Error + */ +@JsonIgnoreProperties(ignoreUnknown = true) +@SuppressFBWarnings(value = { "UWF_UNWRITTEN_FIELD", "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR" }, + justification = "Populated via Jackson deserialization") +public class GHGraphQLError { + + /** + * Source location of an error inside the GraphQL document. + */ + @JsonIgnoreProperties(ignoreUnknown = true) + @SuppressFBWarnings(value = { "UWF_UNWRITTEN_FIELD", "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR" }, + justification = "Populated via Jackson deserialization") + public static class Location { + + private int column; + + private int line; + + /** + * Default constructor used by Jackson. + */ + public Location() { + } + + /** + * Get the column index of the location, starting at 1. + * + * @return the column index + */ + public int getColumn() { + return column; + } + + /** + * Get the line index of the location, starting at 1. + * + * @return the line index + */ + public int getLine() { + return line; + } + } + + private Map extensions; + + private List locations; + + private String message; + + private List path; + + private String type; + + /** + * Default constructor used by Jackson. + */ + public GHGraphQLError() { + } + + /** + * Get the extensions object as defined by the GraphQL spec. GitHub may include custom keys here, e.g. error code or + * documentation URL. May be {@code null} if not provided. + * + * @return the extensions map, or {@code null} + */ + @CheckForNull + public Map getExtensions() { + return extensions == null ? null : Collections.unmodifiableMap(extensions); + } + + /** + * Get the source locations associated with this error. Each entry points to a line/column in the GraphQL document. + * May be {@code null} if not provided. + * + * @return the list of locations, or {@code null} + */ + @CheckForNull + public List getLocations() { + return locations == null ? null : Collections.unmodifiableList(locations); + } + + /** + * Get the human-readable error message. Per the GraphQL spec this field is always present. + * + * @return the message + */ + public String getMessage() { + return message; + } + + /** + * Get the response path that failed. Each segment is either a {@link String} field name or an {@link Integer} list + * index, per the GraphQL spec. May be {@code null} if not provided. + * + * @return the path elements, or {@code null} + */ + @CheckForNull + public List getPath() { + return path == null ? null : Collections.unmodifiableList(path); + } + + /** + * Get the error type as classified by the server. GitHub commonly uses values such as {@code FORBIDDEN}, + * {@code NOT_FOUND}, {@code RATE_LIMITED}, or {@code UNPROCESSABLE}. This is a GitHub extension to the GraphQL spec + * and may be {@code null}. + * + * @return the error type, or {@code null} + */ + @CheckForNull + public String getType() { + return type; + } +} diff --git a/src/main/java/org/kohsuke/github/internal/graphql/response/GHGraphQLResponse.java b/src/main/java/org/kohsuke/github/internal/graphql/response/GHGraphQLResponse.java index 07d012caee..d2784eb04a 100644 --- a/src/main/java/org/kohsuke/github/internal/graphql/response/GHGraphQLResponse.java +++ b/src/main/java/org/kohsuke/github/internal/graphql/response/GHGraphQLResponse.java @@ -8,6 +8,9 @@ import java.util.List; import java.util.stream.Collectors; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; + /** * A response of GraphQL. *

@@ -33,27 +36,15 @@ public static class ObjectResponse extends GHGraphQLResponse { */ @JsonCreator @SuppressFBWarnings(value = { "EI_EXPOSE_REP2" }, justification = "Spotbugs also doesn't like this") - public ObjectResponse(@JsonProperty("data") Object data, @JsonProperty("errors") List errors) { + public ObjectResponse(@JsonProperty("data") Object data, @JsonProperty("errors") List errors) { super(data, errors); } } - /** - * A error of GraphQL response. Minimum implementation for GraphQL error. - */ - @SuppressFBWarnings(value = { "UWF_UNWRITTEN_FIELD", "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR" }, - justification = "JSON API") - private static class GraphQLError { - private String message; - - public String getMessage() { - return message; - } - } - private final T data; - private final List errors; + @Nonnull + private final List errors; /** * GHGraphQLResponse constructor @@ -65,38 +56,73 @@ public String getMessage() { */ @JsonCreator @SuppressFBWarnings(value = { "EI_EXPOSE_REP2" }, justification = "Spotbugs also doesn't like this") - public GHGraphQLResponse(@JsonProperty("data") T data, @JsonProperty("errors") List errors) { - if (errors == null) { - errors = Collections.emptyList(); - } + public GHGraphQLResponse(@JsonProperty("data") T data, @JsonProperty("errors") List errors) { this.data = data; - this.errors = Collections.unmodifiableList(errors); + this.errors = errors == null ? Collections.emptyList() : Collections.unmodifiableList(errors); + } + + /** + * Build a human-readable summary that lists every error message returned by the server. + * + * @param prefix + * short headline prepended before the bullet list + * @return the multi-line summary + */ + public String buildErrorSummary(String prefix) { + return errors.stream() + .map(GHGraphQLError::getMessage) + .collect(Collectors.joining("\n - ", prefix + ":\n - ", "")); } /** * Get response data. * * @return GraphQL success response + * @throws RuntimeException + * if the response carried errors. The exception message lists each error message. */ public T getData() { if (!isSuccessful()) { - throw new RuntimeException("Response not successful, data invalid"); + throw new RuntimeException(buildErrorSummary("Response not successful, data invalid")); } return data; } + /** + * Get response data, including any partial data the server returned alongside errors. Unlike {@link #getData()}, + * this method never throws. + * + * @return the data payload, or {@code null} if absent + */ + @CheckForNull + public T getDataUnchecked() { + return data; + } + /** * Get response error message. * * @return GraphQL error messages from Github Response. Empty list when no errors occurred. */ public List getErrorMessages() { - return errors.stream().map(GraphQLError::getMessage).collect(Collectors.toList()); + return errors.stream().map(GHGraphQLError::getMessage).collect(Collectors.toList()); + } + + /** + * Get the structured GraphQL errors returned by the server. + * + * @return the errors, never {@code null}; empty when the response succeeded + */ + @Nonnull + @SuppressFBWarnings(value = "EI_EXPOSE_REP", + justification = "errors list is wrapped with Collections.unmodifiableList in the constructor") + public List getErrors() { + return errors; } /** - * Is response succesful. + * Is response successful. * * @return request is succeeded. True when error list is empty. */ diff --git a/src/main/resources/META-INF/native-image/org.kohsuke/github-api/reflect-config.json b/src/main/resources/META-INF/native-image/org.kohsuke/github-api/reflect-config.json index 52af02a2a4..7b7f40d9ef 100644 --- a/src/main/resources/META-INF/native-image/org.kohsuke/github-api/reflect-config.json +++ b/src/main/resources/META-INF/native-image/org.kohsuke/github-api/reflect-config.json @@ -6930,7 +6930,22 @@ "allDeclaredClasses": true }, { - "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLResponse", + "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLError", + "allPublicFields": true, + "allDeclaredFields": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredConstructors": true, + "allPublicConstructors": true, + "allDeclaredConstructors": true, + "queryAllPublicMethods": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredMethods": true, + "allPublicClasses": true, + "allDeclaredClasses": true + }, + { + "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLError$Location", "allPublicFields": true, "allDeclaredFields": true, "queryAllPublicConstructors": true, @@ -6945,7 +6960,7 @@ "allDeclaredClasses": true }, { - "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLResponse$GraphQLError", + "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLResponse", "allPublicFields": true, "allDeclaredFields": true, "queryAllPublicConstructors": true, diff --git a/src/main/resources/META-INF/native-image/org.kohsuke/github-api/serialization-config.json b/src/main/resources/META-INF/native-image/org.kohsuke/github-api/serialization-config.json index b4c8f92359..29ad7d73d4 100644 --- a/src/main/resources/META-INF/native-image/org.kohsuke/github-api/serialization-config.json +++ b/src/main/resources/META-INF/native-image/org.kohsuke/github-api/serialization-config.json @@ -1389,10 +1389,13 @@ "name": "org.kohsuke.github.GHPullRequest$EnablePullRequestAutoMergeResponse$EnablePullRequestAutoMerge$EnablePullRequestAutoMergePullRequest" }, { - "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLResponse" + "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLError" + }, + { + "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLError$Location" }, { - "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLResponse$GraphQLError" + "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLResponse" }, { "name": "org.kohsuke.github.internal.graphql.response.GHGraphQLResponse$ObjectResponse" diff --git a/src/test/java/org/kohsuke/github/GHPullRequestTest.java b/src/test/java/org/kohsuke/github/GHPullRequestTest.java index aa1aa231f4..118857d291 100644 --- a/src/test/java/org/kohsuke/github/GHPullRequestTest.java +++ b/src/test/java/org/kohsuke/github/GHPullRequestTest.java @@ -4,6 +4,7 @@ import org.junit.Before; import org.junit.Test; import org.kohsuke.github.GHPullRequest.AutoMerge; +import org.kohsuke.github.internal.graphql.response.GHGraphQLError; import java.io.IOException; import java.time.Instant; @@ -332,8 +333,21 @@ public void enablePullRequestAutoMergeFailure() throws IOException { commitTitle, expectedCommitHeadOid, null); - } catch (IOException e) { + fail("Expected GHGraphQLException"); + } catch (GHGraphQLException e) { + assertThat(e.getMessage(), containsString("Request failed due to following response errors")); assertThat(e.getMessage(), containsString("does not have a verified email")); + + assertThat(e.getErrors(), hasSize(1)); + GHGraphQLError error = e.getErrors().get(0); + assertThat(error.getType(), equalTo("UNPROCESSABLE")); + assertThat(error.getMessage(), containsString("does not have a verified email")); + assertThat(error.getPath(), contains((Object) "enablePullRequestAutoMerge")); + assertThat(error.getLocations(), hasSize(1)); + assertThat(error.getLocations().get(0).getLine(), equalTo(1)); + assertThat(error.getLocations().get(0).getColumn(), equalTo(28)); + + assertThat(e.getQuery(), containsString("enablePullRequestAutoMerge")); } } diff --git a/src/test/java/org/kohsuke/github/internal/graphql/response/GHGraphQLResponseMockTest.java b/src/test/java/org/kohsuke/github/internal/graphql/response/GHGraphQLResponseMockTest.java index 965e8fc7c6..9807650f61 100644 --- a/src/test/java/org/kohsuke/github/internal/graphql/response/GHGraphQLResponseMockTest.java +++ b/src/test/java/org/kohsuke/github/internal/graphql/response/GHGraphQLResponseMockTest.java @@ -8,11 +8,18 @@ import org.junit.jupiter.api.Test; import java.util.List; +import java.util.Map; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; /** * Test GHGraphQLResponse's methods @@ -30,7 +37,52 @@ private GHGraphQLResponse convertJsonToGraphQLResponse(String json) thro } /** - * Test get data throws exception when response means error + * Test the {@link GHGraphQLResponse#buildErrorSummary} helper renders one error per line with the configured + * prefix. + * + * @throws JsonProcessingException + * Json parse exception + */ + @Test + void buildErrorSummaryListsAllErrors() throws JsonProcessingException { + String json = "{\"data\": null, \"errors\": [" + "{\"message\": \"first failure\"}," + + "{\"message\": \"second failure\"}" + "]}"; + + GHGraphQLResponse response = convertJsonToGraphQLResponse(json); + + String summary = response.buildErrorSummary("Request failed"); + assertThat(summary, equalTo("Request failed:\n - first failure\n - second failure")); + } + + /** + * Test partial-success: GraphQL allows {@code data} and {@code errors} to coexist. + * {@link GHGraphQLResponse#getData} still throws because errors are present, but + * {@link GHGraphQLResponse#getDataUnchecked} exposes the partial payload. + * + * @throws JsonProcessingException + * Json parse exception + */ + @Test + void exposesPartialDataAlongsideErrors() throws JsonProcessingException { + String json = "{\"data\": {\"repository\": {\"name\": \"probot\"}}," + + "\"errors\": [{\"message\": \"`invalid cursor` does not appear to be a valid cursor.\"}]}"; + + GHGraphQLResponse response = convertJsonToGraphQLResponse(json); + + assertThat(response.isSuccessful(), is(false)); + assertThat(response.getDataUnchecked(), notNullValue()); + assertThat(response.getErrorMessages(), hasSize(1)); + + try { + response.getData(); + } catch (RuntimeException e) { + assertThat(e.getMessage(), containsString("does not appear to be a valid cursor")); + } + } + + /** + * Test get data throws exception when response means error and the message lists every error message returned by + * the server. * * @throws JsonProcessingException * Json parse exception @@ -49,11 +101,12 @@ void getDataFailure() throws JsonProcessingException { response.getData(); } catch (RuntimeException e) { assertThat(e.getMessage(), containsString("Response not successful, data invalid")); + assertThat(e.getMessage(), containsString("does not have a verified email")); } } /** - * Test getErrorMessages throws exception when response means not error + * Test getErrorMessages returns an empty list when the response has no errors. * * @throws JsonProcessingException * Json parse exception @@ -70,4 +123,76 @@ void getErrorMessagesFailure() throws JsonProcessingException { assertThat(errorMessages, is(empty())); } + /** + * Test that unknown fields on the error object don't break deserialization, ensuring forward compatibility with + * server-side additions. + * + * @throws JsonProcessingException + * Json parse exception + */ + @Test + void ignoresUnknownErrorFields() throws JsonProcessingException { + String json = "{\"data\": null, \"errors\": [{" + "\"message\": \"oops\"," + + "\"futureField\": \"something the client does not know about yet\"" + "}]}"; + + GHGraphQLResponse response = convertJsonToGraphQLResponse(json); + + assertThat(response.getErrors(), hasSize(1)); + assertThat(response.getErrors().get(0).getMessage(), equalTo("oops")); + } + + /** + * Test that all GraphQL spec fields (type, message, path, locations, extensions) are surfaced when the server + * returns the full error structure used by GitHub's GraphQL API. + * + * @throws JsonProcessingException + * Json parse exception + */ + @Test + void parsesFullGraphQLErrorFields() throws JsonProcessingException { + String json = "{\"data\": null, \"errors\": [{" + "\"type\": \"RATE_LIMITED\"," + + "\"message\": \"API rate limit exceeded.\"," + + "\"path\": [\"repository\", \"pullRequests\", 3, \"author\"]," + + "\"locations\": [{\"line\": 7, \"column\": 11}]," + + "\"extensions\": {\"code\": \"RATE_LIMITED\", \"resetAt\": \"2026-01-01T00:00:00Z\"}" + "}]}"; + + GHGraphQLResponse response = convertJsonToGraphQLResponse(json); + + assertThat(response.isSuccessful(), is(false)); + assertThat(response.getErrors(), hasSize(1)); + + GHGraphQLError error = response.getErrors().get(0); + assertThat(error.getType(), equalTo("RATE_LIMITED")); + assertThat(error.getMessage(), equalTo("API rate limit exceeded.")); + assertThat(error.getPath(), contains((Object) "repository", "pullRequests", 3, "author")); + assertThat(error.getLocations(), hasSize(1)); + assertThat(error.getLocations().get(0).getLine(), equalTo(7)); + assertThat(error.getLocations().get(0).getColumn(), equalTo(11)); + + Map extensions = error.getExtensions(); + assertThat(extensions, notNullValue()); + assertThat(extensions, hasEntry("code", "RATE_LIMITED")); + assertThat(extensions, hasEntry("resetAt", "2026-01-01T00:00:00Z")); + } + + /** + * Test the spec-minimum case where only {@code message} is present. All other fields must be null and not throw on + * access. + * + * @throws JsonProcessingException + * Json parse exception + */ + @Test + void parsesMinimalGraphQLError() throws JsonProcessingException { + String json = "{\"data\": null, \"errors\": [{\"message\": \"oops\"}]}"; + + GHGraphQLResponse response = convertJsonToGraphQLResponse(json); + + GHGraphQLError error = response.getErrors().get(0); + assertThat(error.getMessage(), equalTo("oops")); + assertThat(error.getType(), is(nullValue())); + assertThat(error.getPath(), is(nullValue())); + assertThat(error.getLocations(), is(nullValue())); + assertThat(error.getExtensions(), is(nullValue())); + } } diff --git a/src/test/resources/no-reflect-and-serialization-list b/src/test/resources/no-reflect-and-serialization-list index 4ad893272c..4c82aaf589 100644 --- a/src/test/resources/no-reflect-and-serialization-list +++ b/src/test/resources/no-reflect-and-serialization-list @@ -7,6 +7,7 @@ org.kohsuke.github.GHDiscussion$Updater org.kohsuke.github.GHException org.kohsuke.github.GHFileNotFoundException org.kohsuke.github.GHGistUpdater +org.kohsuke.github.GHGraphQLException org.kohsuke.github.GHHooks org.kohsuke.github.GHHooks$Context org.kohsuke.github.GHHooks$OrgContext