Skip to content

fix: makeRequest() silently swallows errors and returns before promise resolves #847

@GaneshPatil7517

Description

@GaneshPatil7517

While reviewing the file loading pipeline in phoenix-ui-components, I found that FileLoaderService.makeRequest() has a broken error-reporting pattern that causes all network and parsing failures to be silently swallowed.

makeRequest() is documented as returning whether an error occurred, but it always returns false - the return false at the bottom executes synchronously before the fetch() promise ever resolves or rejects:

makeRequest(urlPath, responseType, onData, options = {}) {
  fetch(urlPath, options)
    .then((res) => res[responseType]())
    .then((data) => {
      if (responseType === 'blob') {
        data.arrayBuffer()
          .then((buf) => this.unzip(buf))   // ← unzip failure not caught
          .then((d) => onData(d));
      } else {
        onData(data);
      }
    })
    .catch((error) => {
      console.error(error);
      return true;   // ← return value is lost inside the promise chain
    });
  return false;      // ← always executes before fetch completes
}

This causes three cascading problems:

  1. Return value is meaningless return true inside .catch() only resolves the promise chain, it doesn't affect the synchronous return false. Callers (e.g., event-data-explorer-dialog.component.ts) that check the return value to set an error flag will never see an error.
  2. Unzip failures are uncaught - The inner .then((buf) => this.unzip(buf)) chain for blob responses has no .catch(), so a corrupt zip file will cause an unhandled promise rejection with no user-facing feedback.
    3.No user notification on network errors - When fetch() fails (network down, 404, CORS), the error is only logged to console. The UI shows no indication of failure — the app appears to load forever.

Impact

  • User loads an event URL → network request fails → console.error() fires (invisible to user) → function returns false → UI thinks load succeeded → no event data appears → user has no idea what happened
  • User loads a corrupt .zip event → unzip throws → unhandled promise rejection → silent failure

Proposed fix
Convert makeRequest() to async/await with proper error propagation:

  • Return a Promise indicating success/failure
  • Add try/catch around the entire flow including unzip
  • Propagate errors so callers can show user-facing feedback
  • Add .catch() to the inner blob/unzip chain as a safety net

Hello @sponce and @EdwardMoyse I'm happy to submit a PR for this.

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions