visgl / loaders.gl

Loaders for big data visualization. Website:
https://loaders.gl
Other
678 stars 187 forks source link

[Bug] TypeError: Failed to execute 'text' on 'Response': body stream already read #3025

Open davidcalhoun opened 1 month ago

davidcalhoun commented 1 month ago

We have a setup where unfortunately empty tiles are returned from the server with a HTTP 500 Internal Server Error status code and non-JSON text message, e.g. Tile 10/229/410 not found in foo - we're still trying to fix this to instead serve HTTP 204 No Content, but this is a separate issue. For context, we're using TileLayer in @deck.gl@9.0.14

Unfortunately trying to load the empty tiles (with HTTP 500 code and non-JSON text) seems to trigger a bad codepath in @loaders.gl@4.2.1:

The line that triggers it the response.text() here (in core/src/lib/utils/response-utils.ts):

  try {
    const contentType = response.headers.get('Content-Type');
    info.reason = contentType?.includes('application/json')
      ? await response.json()
      : response.text();
  } catch (error) {
    // eslint forbids return in a finally statement, so we just catch here
  }
ibgreen commented 1 month ago

@davidcalhoun Thanks for reporting, putting up a fix.

It would be good to check response status already in the TileLayer and avoid parsing the Response body there so that we can handle missing tiles correctly and take full advantage of error handling logic in loaders.gl.

So it would be interesting to see the stack trace that leads here if you have - just in case it pinpoints things.

davidcalhoun commented 1 month ago

@ibgreen Great, thank you for the quick response!

Here's the stack trace - I added some annotations so you can see the line of code it's traced to:

response-utils.js:83 Uncaught (in promise) TypeError: Failed to execute 'text' on 'Response': body stream already read
    at y (response-utils.js:83:1)  // `: response.text();`
    at m (response-utils.js:54:1) // `const error = await getResponseError(response);`
    at Y (get-data.js:58:15) // `await checkResponse(response);`
    at eu (parse.js:66:18) // `data = await getArrayBufferOrStringFromData(data, loader, options);`
    at el (parse.js:51:1) // `return await parseWithLoader(loader, data, options, context);`
    at async ec (load.js:39:1) // `? await parse(data, resolvedLoaders, resolvedOptions) // loader array overload`
    at async nF._loadData (tile-2d-header.js:81:1) // (deck.gl) `tileData = await getData({ index, id, bbox, userData, zoom, signal });`
ibgreen commented 1 month ago

That is what I was looking for. At first blush, it looks like parse() here in loaders.gl doesn't check the response status before trying to read the data.

ibgreen commented 1 month ago

Fix landed but unfortunately ran into some publishing issues on 4.2 branch... Apologies, will look again when I have more time