Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Nov 6, 2018. It is now read-only.

chore: suppress connection error when disabling an extension#91

Merged
chrismwendt merged 1 commit into
mastersourcegraph/sourcegraph-extension-api:masterfrom
suppress-disconnect-errorsourcegraph/sourcegraph-extension-api:suppress-disconnect-errorCopy head branch name to clipboard
Oct 23, 2018
Merged

chore: suppress connection error when disabling an extension#91
chrismwendt merged 1 commit into
mastersourcegraph/sourcegraph-extension-api:masterfrom
suppress-disconnect-errorsourcegraph/sourcegraph-extension-api:suppress-disconnect-errorCopy head branch name to clipboard

Conversation

@chrismwendt

@chrismwendt chrismwendt commented Oct 22, 2018

Copy link
Copy Markdown

Previously, an unhandled promise rejection could occur when disabling a Sourcegraph extension. Here's a rough sequence of events that could trigger it:

  • The user disables an extension in the UI
  • The configuration gets updated with "foo-extension": false
  • sourcegraph-extension-api sends a request over JSON-RPC (which would eventually notify foo-extension of this configuration change)
  • Before the response comes back, e-c-c removes foo-extension from the list of active extensions and sourcegraph-extension-api unsubscribes the connection
  • The in-flight responses are rejected
  • The rejected promise turns into the console error we see

All I did was suppress the error, because this seems like a case that "shouldn't happen". It's kind of like we're sending SIGTERM to the extension but not waiting 500ms for the extension to exit.

A more involved solution could be: don't send a configuration update to an extension if the only change was the enablement state.

Another solution could be: separate the concepts of enablement state and configuration.

See https://github.com/sourcegraph/sourcegraph/issues/373

@chrismwendt chrismwendt requested a review from sqs October 22, 2018 23:27
@sourcegraph sourcegraph deleted a comment from codecov Bot Oct 22, 2018
@sourcegraph sourcegraph deleted a comment from codecov Bot Oct 22, 2018
@chrismwendt chrismwendt merged commit 64b11f6 into master Oct 23, 2018
@chrismwendt chrismwendt deleted the suppress-disconnect-error branch October 23, 2018 02:23
@sourcegraph-bot

Copy link
Copy Markdown

🎉 This PR is included in version 18.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.