Skip to content

Commit

Permalink
fix: microsoft need not be strict in response-body expectation (#1417)
Browse files Browse the repository at this point in the history
  • Loading branch information
jzacsh authored Dec 20, 2024
1 parent 9e967cb commit dfd2342
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,19 @@ public abstract class MicrosoftApiResponse {
/** HTTP body of the response if any was present. */
public abstract Optional<String> body();

/** Exception that occurred when trying to read the HTTP body of the response. */
public abstract Optional<IOException> bodyException();

/**
* Builds from key fields within an HTTP response, closing said response
*
* <p>If the body is a requirement and you need to fail-fast for bad response-bodies, then make
* sure you're calling an introspective method that tries to actually utiilze the body, eg: {@link
* getJsonValue} (or add a new one). By default, because bodies are not always provided or needed,
* this construction method tries to save the response-body but stores an error in {@link
* bodyException} (which will be included in the usual exception output via {@link
* throwDtpException} and co).
*
* <p>Warning: this loads the entire response body to memory, as we assume all Microsoft API
* responses we're dealing with are (at the most complex end) just JSON responses intended to be
* parsed (as opposed to say, streams of an arbitrarily-large file's bytes).
Expand All @@ -81,15 +91,13 @@ public static MicrosoftApiResponse ofResponse(Response response) throws IOExcept
final String body;
try {
body = response.body().string();
if (!Strings.isNullOrEmpty(body)) {
builder.setBody(body);
}
} catch (IOException e) {
throw builder
.build()
.toIoException("bug? loading body from response shouldn't hit IOException, but did", e);
builder.setBodyException(e);
}

if (!Strings.isNullOrEmpty(body)) {
builder.setBody(body);
}
response.close(); // only close if we _had_ a body field to read
}

Expand All @@ -104,6 +112,8 @@ public abstract static class Builder {

public abstract Builder setBody(String body);

public abstract Builder setBodyException(IOException bodyException);

public abstract MicrosoftApiResponse build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,9 @@ private Request.Builder buildCreateUploadSessionPath(
* <ul>
* <li>https://learn.microsoft.com/en-us/graph/api/driveitem-createuploadsession?view=graph-rest-1.0#upload-bytes-to-the-upload-session
* </ul>
*
* Returns a response, which is only expected to have a response-body in the
* event that this is the final chunk.
*/
private MicrosoftApiResponse uploadChunk(
DataChunk chunk, String photoUploadUrl, long totalFileSize, String mediaType)
Expand Down Expand Up @@ -390,7 +393,10 @@ private Pair<Request, MicrosoftApiResponse> tryWithCreds(Request.Builder request
* standard token-refresh retry options.
*
* <p>Prefer {@link tryWithCredsOrFail(Request.Builder, String, String)} if you ultimately only
* care about a particular JSON key in the response body.
* care about a particular JSON key in the response body. If you don't
* need/expect a response body (or don't know yet) then this is the right
* method to call instead (and then call {@link
* MicrosoftApiResponse#getJsonValue} yourself later).
*
* <p>Example usage:
*
Expand Down

0 comments on commit dfd2342

Please sign in to comment.