Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

fix: handle parsing & primary error properly #235

Merged
merged 1 commit into from
May 28, 2018

Conversation

vigneshshanmugam
Copy link
Collaborator

@vigneshshanmugam vigneshshanmugam commented May 28, 2018

  • If we respond with a empty message with Content-Type: 'text/html', the browser will throw error stating as This page isn’t working losing the original error page.
  • Now we respond with 500 status code and Internal Server Error as the response when there is a parse error or when there is no fallback provided for primary fragment.

This is a very old bug and it was introduced when we started supporting fallbacks and I noticed after we started the Opentracing instrumentation, so Thanks to @lmineiro 😄

@codecov
Copy link

codecov bot commented May 28, 2018

Codecov Report

Merging #235 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #235      +/-   ##
==========================================
+ Coverage   99.05%   99.06%   +<.01%     
==========================================
  Files          16       16              
  Lines         638      639       +1     
  Branches      117      117              
==========================================
+ Hits          632      633       +1     
  Misses          6        6
Impacted Files Coverage Δ
lib/request-handler.js 98.97% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 345caa3...5d1ddeb. Read the comment docs.

@vigneshshanmugam
Copy link
Collaborator Author

👍

1 similar comment
@DeTeam
Copy link
Contributor

DeTeam commented May 28, 2018

👍

@vigneshshanmugam vigneshshanmugam merged commit 88c5850 into master May 28, 2018
@vigneshshanmugam vigneshshanmugam deleted the fix-response-msg branch May 28, 2018 14:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants