Read response body closer to the fetch call (#5562)

This commit is contained in:
Kamil Kisiela 2024-08-29 19:11:06 +02:00 committed by GitHub
parent a52cd88223
commit 0ba8272b4c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 40 additions and 20 deletions

View file

@ -14,7 +14,9 @@ export type GetArtifactActionFn = (
artifactType: ArtifactsType,
eTag: string | null,
) => Promise<
{ type: 'notModified' } | { type: 'notFound' } | { type: 'response'; response: Response }
| { type: 'notModified' }
| { type: 'notFound' }
| { type: 'response'; status: Response['status']; headers: Response['headers']; body: string }
>;
type ArtifactRequestHandler = {
@ -161,7 +163,9 @@ export const createArtifactRequestHandler = (deps: ArtifactRequestHandler) => {
}
if (result.type === 'response') {
const etag = result.response.headers.get('etag');
const etag = result.headers.get('etag');
const text = result.body;
if (params.artifactType === 'metadata') {
// To not change a lot of logic and still reuse the etag bits, we
// fetch metadata using the redirect location.
@ -172,18 +176,16 @@ export const createArtifactRequestHandler = (deps: ArtifactRequestHandler) => {
// and we're fetching the artifact from within the private network.
// If they are the same, private and public locations will be the same.
const body = await result.response.clone().text();
// Metadata in SINGLE projects is only Mesh's Metadata, and it always defines _schema
const isMeshArtifact = body.includes(`"#/definitions/_schema"`);
const hasTopLevelArray = body.startsWith('[') && body.endsWith(']');
const isMeshArtifact = text.includes(`"#/definitions/_schema"`);
const hasTopLevelArray = text.startsWith('[') && text.endsWith(']');
// Mesh's Metadata shared by Mesh is always an object.
// The top-level array was caused #3291 and fixed now, but we still need to handle the old data.
if (isMeshArtifact && hasTopLevelArray) {
return createResponse(
analytics,
body.substring(1, body.length - 1),
text.substring(1, text.length - 1),
{
status: 200,
headers: {
@ -197,9 +199,6 @@ export const createArtifactRequestHandler = (deps: ArtifactRequestHandler) => {
}
}
breadcrumb('Reading response.text()');
const text = await result.response.text();
breadcrumb('Returning OK 200');
return createResponse(
analytics,
text,

View file

@ -272,7 +272,12 @@ export class ArtifactStorageReader {
if (response.status === 200) {
return {
type: 'response',
response,
status: response.status,
headers: response.headers,
body: await response.text().catch(error => {
this.breadcrumb('Error reading response body: ' + String(error));
return Promise.reject(error);
}),
} as const;
}

View file

@ -287,7 +287,7 @@ export function createRequestHandler(deps: RequestHandlerDependencies) {
const rawValueAction = await deps.getArtifactAction(targetId, null, storageKeyType, null);
if (rawValueAction.type === 'response') {
const rawValue = await rawValueAction.response.text();
const rawValue = rawValueAction.body;
const etag = await createETag(`${kvStorageKey}|${rawValue}`);
const ifNoneMatch = request.headers.get('if-none-match');

View file

@ -93,7 +93,9 @@ describe('CDN Worker', () => {
return map.has(`target:${targetId}:${artifactType}`)
? {
type: 'response',
response: new Response(map.get(`target:${targetId}:${artifactType}`)),
status: 200,
body: map.get(`target:${targetId}:${artifactType}`),
headers: new Headers(),
}
: {
type: 'notFound',
@ -176,7 +178,9 @@ describe('CDN Worker', () => {
return map.has(`target:${targetId}:${artifactType}`)
? {
type: 'response',
response: new Response(map.get(`target:${targetId}:${artifactType}`)),
status: 200,
headers: new Headers(),
body: map.get(`target:${targetId}:${artifactType}`),
}
: {
type: 'notFound',
@ -275,7 +279,9 @@ describe('CDN Worker', () => {
return map.has(`target:${targetId}:${artifactType}`)
? {
type: 'response',
response: new Response(map.get(`target:${targetId}:${artifactType}`)),
status: 200,
headers: new Headers(),
body: map.get(`target:${targetId}:${artifactType}`),
}
: {
type: 'notFound',
@ -358,7 +364,9 @@ describe('CDN Worker', () => {
return map.has(`target:${targetId}:${artifactType}`)
? {
type: 'response',
response: new Response(map.get(`target:${targetId}:${artifactType}`)),
status: 200,
headers: new Headers(),
body: map.get(`target:${targetId}:${artifactType}`),
}
: {
type: 'notFound',
@ -447,7 +455,9 @@ describe('CDN Worker', () => {
return map.has(`target:${targetId}:${artifactType}`)
? {
type: 'response',
response: new Response(map.get(`target:${targetId}:${artifactType}`)),
status: 200,
headers: new Headers(),
body: map.get(`target:${targetId}:${artifactType}`),
}
: {
type: 'notFound',
@ -534,7 +544,9 @@ describe('CDN Worker', () => {
return map.has(`target:${targetId}:${artifactType}`)
? {
type: 'response',
response: new Response(map.get(`target:${targetId}:${artifactType}`)),
status: 200,
headers: new Headers(),
body: map.get(`target:${targetId}:${artifactType}`),
}
: {
type: 'notFound',
@ -706,7 +718,9 @@ describe('CDN Worker', () => {
return map.has(`target:${targetId}:${artifactType}`)
? {
type: 'response',
response: new Response(map.get(`target:${targetId}:${artifactType}`)),
status: 200,
headers: new Headers(),
body: map.get(`target:${targetId}:${artifactType}`),
}
: {
type: 'notFound',
@ -761,7 +775,9 @@ describe('CDN Worker', () => {
return map.has(`target:${targetId}:${artifactType}`)
? {
type: 'response',
response: new Response(map.get(`target:${targetId}:${artifactType}`)),
status: 200,
headers: new Headers(),
body: map.get(`target:${targetId}:${artifactType}`),
}
: {
type: 'notFound',