Skip to content

Remove fallback call for redeeming sharing link in ODSP driver which was just added as a precaution when main network was added. #24790

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

Merged
merged 1 commit into from
Jun 9, 2025
Merged
Changes from all 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
44 changes: 16 additions & 28 deletions packages/drivers/odsp-driver/src/fetchSnapshot.ts
Original file line number Diff line number Diff line change
@@ -27,7 +27,6 @@ import {
ITelemetryLoggerExt,
PerformanceEvent,
isFluidError,
loggerToMonitoringContext,
wrapError,
} from "@fluidframework/telemetry-utils/internal";
import { v4 as uuid } from "uuid";
@@ -215,7 +214,7 @@ async function redeemSharingLink(
{
eventName: "RedeemShareLink",
},
async () => {
async (event) => {
assert(
!!odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem,
0x1ed /* "Share link should be present" */,
@@ -246,34 +245,23 @@ async function redeemSharingLink(
});
}

const disableUsingTenantDomain = loggerToMonitoringContext(logger).config.getBoolean(
"Fluid.Driver.Odsp.DisableUsingTenantDomainForSharesApi",
);
const details = JSON.stringify({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit (optional): You don't need JSON.stringify here, the logger will do that for you before sending.

length: redeemUrl?.length,
shareLinkUrlLength: odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem.length,
queryParamsLength: new URL(odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem).search
.length,
useHeaders: true,
});
// There is an issue where if we use the siteUrl in /shares, then the allowed length of url is just a few hundred characters(300-400)
// and we fail to do the redeem. But if we use the tenant domain in the url, then the allowed length becomes 2048. So, first
// construct the url for /shares using tenant domain but to be on safer side, fallback to using the siteUrl. We get tenant domain
// by getting origin of the siteUrl.
if (!disableUsingTenantDomain) {
try {
await callSharesAPI(new URL(odspResolvedUrl.siteUrl).origin);
return;
} catch (error) {
logger.sendTelemetryEvent(
{
eventName: "ShareLinkRedeemFailedWithTenantDomain",
details: JSON.stringify({
length: redeemUrl?.length,
shareLinkUrlLength: odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem.length,
queryParamsLength: new URL(odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem)
.search.length,
useHeaders: true,
}),
},
error,
);
}
// and we fail to do the redeem. But if we use the tenant domain in the url, then the allowed length becomes 2048. So,
// construct the url for /shares using tenant domain. We get tenant domain by getting origin of the siteUrl.
try {
await callSharesAPI(new URL(odspResolvedUrl.siteUrl).origin);
event.end({ details });
} catch (error) {
event.cancel({ details }, error);
throw error;
}
await callSharesAPI(odspResolvedUrl.siteUrl);
},
);
}
46 changes: 0 additions & 46 deletions packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts
Original file line number Diff line number Diff line change
@@ -567,52 +567,6 @@ describe("Tests1 for snapshot fetch", () => {
]),
);
});

it("RedeemFallback behavior when fallback succeeds with using siteUrl", async () => {
resolved.shareLinkInfo = {
sharingLinkToRedeem: "https://microsoft.sharepoint-df.com/sharelink",
};
hostPolicy.enableRedeemFallback = true;

const snapshot: ISnapshot = {
blobContents,
snapshotTree: snapshotTreeWithGroupId,
ops: [],
latestSequenceNumber: 0,
sequenceNumber: 0,
snapshotFormatV: 1,
};
const response = (await createResponse(
{ "x-fluid-epoch": "epoch1", "content-type": "application/ms-fluid" },
convertToCompactSnapshot(snapshot),
200,
)) as unknown as Response;

await assert.doesNotReject(
async () =>
mockFetchMultiple(
async () => service.getSnapshot({}),
[
notFound,
notFound,
async (): Promise<MockResponse> => okResponse({}, {}),
async (): Promise<Response> => {
return response;
},
],
),
"Should succeed",
);
assert(
mockLogger.matchEvents([
{ eventName: "TreesLatest_cancel", shareLinkPresent: true },
{ eventName: "ShareLinkRedeemFailedWithTenantDomain", statusCode: 404 },
{ eventName: "RedeemShareLink_end" },
{ eventName: "RedeemFallback", errorType: "fileNotFoundOrAccessDeniedError" },
{ eventName: "TreesLatest_end" },
]),
);
});
});

const snapshotTreeWithGroupId: ISnapshotTree = {
Loading
Oops, something went wrong.