Cannot set headers after they are sent to the client error occurs a lot

1070rik commented 4 years ago

Current behavior

Currently in our bug tracking software we see this error a lot. (47k occurrences in like 3 weeks) It seems to happen when a 404 or error page gets rendered according to the error.

Don't know if I'm correct but if you do a res.redirect or something like that, you need to do a return after that so it doesn't try to do another redirect? So if you would want to do a 404 or 500 redirect the errorHandler in server.ts would look like this:

  const errorHandler = err => {
    if (err && err.code === 404) {
      if (NOT_ALLOWED_SSR_EXTENSIONS_REGEX.test(req.url)) {
        apiStatus(res, 'Vue Storefront: Resource is not found', 404)
        console.error(`Resource is not found : ${req.url}`)
        return;
      } else {
        res.redirect('/page-not-found')
        console.error(`Redirect for resource not found : ${req.url}`)
        return;
      }
    } else {
      res.redirect('/error')
      console.error(`Error during render : ${req.url}`)
      console.error(err)
      return;
    }
  }

I don't have a lot of experience with express so I might be wrong.

Expected behavior

It shouldn't happen at all.

Steps to reproduce the issue

Run VSF in a production environment.

Repository

Can you handle fixing this bug by yourself?

[ ] YES
[X] NO

Which Release Cycle state this refers to? Info for developer.

Pick one option.

[ ] This is a bug report for test version on https://test.storefrontcloud.io - In this case Developer should create branch from develop branch and create Pull Request 2. Feature / Improvement back to develop.
[ ] This is a bug report for current Release Candidate version on https://next.storefrontcloud.io - In this case Developer should create branch from release branch and create Pull Request 3. Stabilisation fix back to release.
[X] This is a bug report for current Stable version on https://demo.storefrontcloud.io and should be placed in next stable version hotfix - In this case Developer should create branch from hotfix or master branch and create Pull Request 4. Hotfix back to hotfix.

Environment details

Browser: Chrome 81.0.4044.92
OS: Mac os 10.15.1
Node: V10.19.0
Code Version: V1.11.0

Additional information

1070rik commented 4 years ago

8 days later this has risen to 74k occurrences. Can someone confirm if the code I wrote is a good temporary fix so I don't have over 100k occurrences next week.

pkarw commented 4 years ago

@gibkigonzo please take a look

gibkigonzo commented 4 years ago

@1070rik Yes, there shouldn't be next call after redirect. Not related, but this is also wrong

      if (typeof afterOutputRenderedResponse.output === 'string') {
        res.end(afterOutputRenderedResponse.output)
      } else if (typeof afterOutputRenderedResponse === 'string') {
        res.end(afterOutputRenderedResponse)
      } else {
        res.end(output)
      }

if afterOutputRenderedResponse is a string then we will get error here.

1070rik commented 4 years ago

@gibkigonzo How would I be able to fix this until 1.11.4 gets released? I fixed the errorhandler method but the error still occurs (probably caused by the afterOutputRenderedResponse part like you mentioned?).

simonmaass commented 4 years ago

i also get these on API side... dunno if releated... https://github.com/DivanteLtd/vue-storefront-api/issues/434

gibkigonzo commented 4 years ago

@1070rik Can you send your config? just the part that you changed (local.json). It will help to reproduce your context and track this problem.

1070rik commented 4 years ago

@gibkigonzo yeah sure. Here you go.

{
  "server": {
    "port": 3007,
    "dynamicConfigReload": true,
    "useOutputCacheTagging": true,
    "useOutputCache": true,
    "invalidateCacheForwarding": true,
    "invalidateCacheForwardUrl": "https://api.sabe-verpakkingen.nl/invalidate?key=******&tag="
  },
  "api": {
    "url": "https://api.sabe-verpakkingen.nl"
  },
  "elasticsearch": {
    "index": "vue_storefront_magento_1",
    "searchableAttributes": {
      "sku": {
        "boost": 10
      },
      "configurable_children.sku": {
        "boost": 3
      },
      "category.name": {
        "boost": 1
      }
    }
  },
  "redis": {
    "host": "localhost",
    "port": 6379,
    "db": 0
  },
  "entities": {
    "category": {
      "includeFields": [
        "id",
        "*.children_data.*",
        "*.name",
        "*.id",
        "*.slug",
        "*.url_path",
        "*.image",
        "*.description",
        "children_count",
        "sku",
        "name",
        "is_active",
        "parent_id",
        "level",
        "url_key",
        "url_path",
        "product_count",
        "path",
        "description",
        "short_description",
        "image",
        "meta_keywords",
        "meta_description",
        "meta_title"
      ]
    },
    "productList": {
      "includeFields": [
        "type_id",
        "sku",
        "product_links",
        "tax_class_id",
        "special_price",
        "special_to_date",
        "special_from_date",
        "name",
        "price",
        "priceInclTax",
        "originalPriceInclTax",
        "originalPrice",
        "specialPriceInclTax",
        "id",
        "image",
        "sale",
        "new",
        "url_path",
        "url_key",
        "status",
        "tier_prices",
        "inhoud",
        "inhoudpapier",
        "configurable_children.sku",
        "configurable_children.price",
        "configurable_children.special_price",
        "configurable_children.priceInclTax",
        "configurable_children.specialPriceInclTax",
        "configurable_children.originalPrice",
        "configurable_children.originalPriceInclTax",
        "configurable_children.afmeting",
        "configurable_children.inhoudpapier",
        "configurable_children.afmeting_overig",
        "configurable_children.formaat",
        "configurable_children.stock.*",
        "description",
        "stock.*"
      ],
      "excludeFields": [
        "configurable_options",
        "sgn",
        "*.sgn",
        "msrp_display_actual_price_type",
        "*.msrp_display_actual_price_type",
        "required_options"
      ]
    }
  },
  "i18n": {
    "defaultCountry": "NL",
    "defaultLanguage": "NL",
    "availableLocale": [
      "nl-NL"
    ],
    "defaultLocale": "nl-NL",
    "currencyCode": "EUR",
    "currencySign": "€",
    "fullCountryName": "Nederland",
    "fullLanguageName": "Nederlands"
  },
  "mailer": {
    "endpoint": {
      "send": "/api/ext/vsf-sparkpost-mail-service/send-email",
      "token": "/api/ext/vsf-sparkpost-mail-service/get-token"
    },
    "contactAddress": "test@mail.com",
    "sendConfirmation": false
  },
  "tax": {
    "defaultCountry": "NL"
  },
  "theme": "sabe",
  "images": {
    "baseUrl": "https://api.sabe-verpakkingen.nl/img/"
  },
  "mollie": {
    "endpoint": "https://api.sabe-verpakkingen.nl/api/ext/vsf-payment-mollie-payments-api",
    "error_url": "default",
    "invalid_payment_status_check_url": "error"
  },
  "delivery-times": {
    "endpoint": "/api/ext/vsf-delivery-time"
  },
  "orders": {
    "payment_methods_mapping": {
      "ideal": "mollie_methods_ideal",
      "sofort": "mollie_methods_sofort",
      "bancontact": "mollie_methods_bancontact",
      "purchaseorder": "purchaseorder"
    }
  },
  "products": {
    "defaultFilters": [
      "color",
      "size",
      "hoogte_dozen"
    ],
    "gallery": {
      "mergeConfigurableChildren": false,
      "imageAttributes": ["image"]
    }
  },
  "query": {
    "newProducts": {
      "filter": []
    },
    "bestSellers": {
      "filter": []
    }
  }
}

vuestorefront / vue-storefront