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

Handle location redirection URL error in getSharingInfo call in odsp driver #21277

Merged
merged 7 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 62 additions & 2 deletions packages/drivers/odsp-driver/src/getFileLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@
import type { ITelemetryBaseProperties } from "@fluidframework/core-interfaces";
import { assert } from "@fluidframework/core-utils/internal";
import { NonRetryableError, runWithRetry } from "@fluidframework/driver-utils/internal";
import { hasRedirectionLocation } from "@fluidframework/odsp-doclib-utils/internal";
import {
IOdspUrlParts,
OdspErrorTypes,
OdspResourceTokenFetchOptions,
TokenFetcher,
} from "@fluidframework/odsp-driver-definitions/internal";
import { ITelemetryLoggerExt, PerformanceEvent } from "@fluidframework/telemetry-utils/internal";
import {
ITelemetryLoggerExt,
PerformanceEvent,
isFluidError,
} from "@fluidframework/telemetry-utils/internal";

import { getUrlAndHeadersWithAuth } from "./getUrlAndHeadersWithAuth.js";
import {
Expand Down Expand Up @@ -55,7 +60,12 @@ export async function getFileLink(
fileLinkCore = await runWithRetry(
async () =>
runWithRetryForCoherencyAndServiceReadOnlyErrors(
async () => getFileLinkCore(getToken, odspUrlParts, logger),
async () =>
getFileLinkWithLocationRedirectionHandling(
getToken,
odspUrlParts,
logger,
),
"getFileLinkCore",
logger,
),
Expand Down Expand Up @@ -95,6 +105,54 @@ export async function getFileLink(
return fileLink;
}

/**
* Handles location redirection while fulfilling the getFilelink call. We don't want browser to handle
* the redirection as the browser will retry with same auth token which will not work as we need app
* to regenerate tokens for the new site domain. So when we will make the network calls below we will set
* the redirect:manual header to manually handle these redirects.
* Refer: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308
* @param getToken - token fetcher to fetch the token.
* @param odspUrlParts - parts of odsp resolved url.
* @param logger - logger to send events.
* @returns Response from the API call.
* @alpha
*/
async function getFileLinkWithLocationRedirectionHandling(
getToken: TokenFetcher<OdspResourceTokenFetchOptions>,
odspUrlParts: IOdspUrlParts,
logger: ITelemetryLoggerExt,
): Promise<string> {
// We can have chains of location redirection one after the other, so have a for loop
// so that we can keep handling the same type of error. Set max number of redirection to 5.
let lastError: unknown;
for (let count = 1; count <= 5; count++) {
try {
return await getFileLinkCore(getToken, odspUrlParts, logger);
} catch (error: unknown) {
lastError = error;
if (
isFluidError(error) &&
error.errorType === OdspErrorTypes.fileNotFoundOrAccessDeniedError &&
hasRedirectionLocation(error) &&
error.redirectLocation !== undefined
) {
const redirectLocation = error.redirectLocation;
logger.sendTelemetryEvent({
jatgarg marked this conversation as resolved.
Show resolved Hide resolved
eventName: "LocationRedirectionErrorForGetOdspFileLink",
retryCount: count,
});
// Generate the new SiteUrl from the redirection location.
const newSiteDomain = new URL(redirectLocation).origin;
const newSiteUrl = `${newSiteDomain}${new URL(odspUrlParts.siteUrl).pathname}`;
odspUrlParts.siteUrl = newSiteUrl;
continue;
}
throw error;
}
}
throw lastError;
}

async function getFileLinkCore(
getToken: TokenFetcher<OdspResourceTokenFetchOptions>,
odspUrlParts: IOdspUrlParts,
Expand Down Expand Up @@ -138,6 +196,7 @@ async function getFileLinkCore(
headers: {
"Content-Type": "application/json;odata=verbose",
"Accept": "application/json;odata=verbose",
"redirect": "manual",
...headers,
},
};
Expand Down Expand Up @@ -219,6 +278,7 @@ async function getFileItemLite(
storageToken,
forceAccessTokenViaAuthorizationHeader,
);
headers.redirect = "manual";
const requestInit = { method: "GET", headers };
const response = await fetchHelper(url, requestInit);
additionalProps = response.propsToLog;
Expand Down
169 changes: 169 additions & 0 deletions packages/drivers/odsp-driver/src/test/getFileLink.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {

describe("getFileLink", () => {
const siteUrl = "https://microsoft.sharepoint-df.com/siteUrl";
const newSiteUrl = "https://microsoft.sharepoint.com/siteUrl";
const driveId = "driveId";
const logger = new MockLogger();
const storageTokenFetcher = async (): Promise<string> => "StorageToken";
Expand Down Expand Up @@ -145,4 +146,172 @@ describe("getFileLink", () => {
"did not retries 5 times",
);
});

it("should handle location redirection once", async () => {
const result = await mockFetchMultiple(
async () =>
getFileLink(
storageTokenFetcher,
{ siteUrl, driveId, itemId: "itemId8" },
logger.toTelemetryLogger(),
),
[
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> => okResponse({}, fileItemResponse),
async (): Promise<MockResponse> =>
okResponse({}, { d: { directUrl: "sharelink" } }),
],
);
assert.strictEqual(
result,
"sharelink",
"File link should match url returned from sharing information",
);
// Should be present in cache now and subsequent calls should fetch from cache.
const sharelink2 = await getFileLink(
storageTokenFetcher,
{ siteUrl, driveId, itemId: "itemId8" },
logger.toTelemetryLogger(),
);
assert.strictEqual(
sharelink2,
"sharelink",
"File link should match url returned from sharing information from cache",
);
});

it("should handle location redirection multiple times", async () => {
const result = await mockFetchMultiple(
async () =>
getFileLink(
storageTokenFetcher,
{ siteUrl, driveId, itemId: "itemId9" },
logger.toTelemetryLogger(),
),
[
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
302,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
307,
),
async (): Promise<MockResponse> => okResponse({}, fileItemResponse),
async (): Promise<MockResponse> =>
okResponse({}, { d: { directUrl: "sharelink" } }),
],
);
assert.strictEqual(
result,
"sharelink",
"File link should match url returned from sharing information",
);
// Should be present in cache now and subsequent calls should fetch from cache.
const sharelink2 = await getFileLink(
storageTokenFetcher,
{ siteUrl, driveId, itemId: "itemId9" },
logger.toTelemetryLogger(),
);
assert.strictEqual(
sharelink2,
"sharelink",
"File link should match url returned from sharing information from cache",
);
});

it("should handle location redirection max 5 times", async () => {
await assert.rejects(
mockFetchMultiple(async () => {
return getFileLink(
storageTokenFetcher,
{ siteUrl, driveId, itemId: "itemId10" },
logger.toTelemetryLogger(),
);
}, [
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
]),
"File link should reject when not found",
);
});
});
1 change: 1 addition & 0 deletions packages/utils/odsp-doclib-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export {
fetchIncorrectResponse,
getSPOAndGraphRequestIdsFromResponse,
hasFacetCodes,
hasRedirectionLocation,
OdspErrorResponse,
OdspErrorResponseInnerError,
OdspRedirectError,
Expand Down
27 changes: 27 additions & 0 deletions packages/utils/odsp-doclib-utils/src/odspErrorUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,19 @@ export function createOdspNetworkError(
const driverProps = { driverVersion, statusCode, ...props };

switch (statusCode) {
// The location of file can move on Spo. If the redirect:manual header is added to network call
// it causes browser to not handle the redirects. Location header in such cases will contain the new location.
// Refer: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308
case 301:
case 302:
case 303:
case 307:
case 308:
redirectLocation = response?.headers.get("Location") ?? undefined;
if (redirectLocation !== undefined) {
error = new OdspRedirectError(errorMessage, redirectLocation, driverProps);
break;
}
case 400:
if (innerMostErrorCode === "fluidInvalidSchema") {
error = new FluidInvalidSchemaError(errorMessage, driverProps);
Expand Down Expand Up @@ -446,3 +459,17 @@ function numberFromHeader(header: string | null): number | undefined {
export function hasFacetCodes(x: any): x is Pick<IOdspErrorAugmentations, "facetCodes"> {
return Array.isArray(x?.facetCodes);
}

/**
* @internal
*/
export function hasRedirectionLocation(
x: unknown,
): x is Pick<IOdspErrorAugmentations, "redirectLocation"> {
return (
x !== null &&
typeof x === "object" &&
"redirectLocation" in x &&
typeof x?.redirectLocation === "string"
);
}
Loading