New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sourcegraph.com experiencing unexpected error every now and then #1760

Closed
vanesa opened this Issue Jan 10, 2019 · 8 comments

Comments

Projects
None yet
5 participants
@vanesa
Copy link
Member

vanesa commented Jan 10, 2019

I've been seeing this error every now and then when using Sourcegraph.com . It will randomly appear when I'm switching from one page to another. It resolves by reloading the page.

image

The console shows this error:
Failed to load resource: the server responded with a status of 404 () originating from a https://sourcegraph.com/.assets/scripts/27-43007cc1aa014fb8b79e.chunk.js

I'll keep the network tab open from now on so I can get that error too, next time this happens.

@vanesa vanesa changed the title sourcegraph.com experiencing unexpected error frequently sourcegraph.com experiencing unexpected error every now and then Jan 10, 2019

@nicksnyder nicksnyder assigned lguychard and unassigned sqs Jan 10, 2019

@nicksnyder nicksnyder added this to the 3.0 milestone Jan 10, 2019

@nicksnyder nicksnyder assigned felixfbecker and unassigned lguychard Jan 10, 2019

@nicksnyder

This comment has been minimized.

Copy link
Member

nicksnyder commented Jan 10, 2019

Can you look into this Felix?

@felixfbecker

This comment has been minimized.

Copy link
Member

felixfbecker commented Jan 10, 2019

I changed the error boundary to show error messages. I don't know exactly how to repro though so can't do anything else rn. Sentry would help here.

@nicksnyder

This comment has been minimized.

Copy link
Member

nicksnyder commented Jan 10, 2019

Have any theory why a JS chunk would 404?

@sqs

This comment has been minimized.

Copy link
Member

sqs commented Jan 10, 2019

If the site was redeployed and you were browsing around.

@nicksnyder

This comment has been minimized.

Copy link
Member

nicksnyder commented Jan 10, 2019

Ok, I could imagine how this could happen if we are lazily loading JS chunks (is that the case?).

If so a reproduction might look like:

  1. Open sourcegraph in a browser tab
  2. Deploy a new version of javascript
  3. Navigate around the app to force it to try to load a new chunk -> this could 404

Is there a way to catch these 404s and handle them by doing a hard page refresh automatically?

@sqs

This comment has been minimized.

Copy link
Member

sqs commented Jan 10, 2019

Yeah we are now, I introduced that recently. This is definitely a problem. We could go back to using a single chunk for now in webpack config (ideally keep the work that refactored the UI to use multiple chunks, but just override webpack in 1 place to tell it to emit a single chunk).

Doing a hard page refresh seems like the best simple option, but it MIGHT be problematic:

  1. if you had state like an unsaved settings editor (then a hard page refresh would lose it) -- although you're unlikely (impossible?) to encounter these 404s when you are in that state
  2. identifying which 404s would be fixed by this reload and which wouldn't may be complicated, and we don't want a reload loop

If someone has a good idea on how to address those 2 things, then 👍 let's do it

@nicksnyder

This comment has been minimized.

Copy link
Member

nicksnyder commented Jan 10, 2019

For 3.0 release I think we should just go back to 1 chunk. I neither saw any recent commits in webpack.config.ts, nor on obvious config to set to achieve the desired behavior. @felixfbecker can you do this?

@sqs

This comment has been minimized.

Copy link
Member

sqs commented Jan 10, 2019

There were no changes to webpack config, it was changes in how we import code. I'll do it (quick change, I have context)

@sqs sqs assigned sqs and unassigned felixfbecker Jan 10, 2019

sqs added a commit that referenced this issue Jan 11, 2019

@sqs sqs added the in-review label Jan 11, 2019

sqs added a commit that referenced this issue Jan 11, 2019

sqs added a commit that referenced this issue Jan 11, 2019

sqs added a commit that referenced this issue Jan 11, 2019

sqs added a commit that referenced this issue Jan 11, 2019

@sqs sqs closed this in #1803 Jan 11, 2019

sqs added a commit that referenced this issue Jan 11, 2019

show "Reload required" error if app JS is redeployed after user's ini…
…tial page load (#1803)

* use contenthash not chunkhash

Webpack recommends using contenthash because it is based on the hash of the actual chunk content, not of the chunk source, so it is more accurate and changes only when a meaningful change has occurred. There is no difference in actual outcomes for our use case currently.

* reset error page when user navigates

This lets the user navigate away from the error. For example, if an error occurs on one tab in the user area, they can click on other tabs and those will work. Previously, the error would remain even when the user attempted to navigate away from it, as long as the error's parent stayed the same.

* show "Reload required" error if app JS is redeployed after user's initial page load

fix #1760
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment