Status codes and well-defined errors in XYZProvider interfaces

[jrieken]

We should explore a list of well defined errors and status codes with which data provider can signal reasons for not producing a result. A sample would be a DefinitionProvider which can produce a result because it lacks configuration or is still fetching required data (d.ts-loading)

[jrieken]

In the long run we should have two concepts (1) provider specific status and (2) language specific status. For (1), and that is short term, a provider could do this

registerCompletionItemProvider('foo-lang', {
  provideCompletionItems(doc, pos, token) {
    if(somePreconditionNotMet) {
      throw new Status('short message', 'long message')
    }
  }
})

The UI side will have these the status errors accordingly. Open questions

• throw/reject vs or-typing?
• Having either status or provider result vs having both status and provider result.

Wrt a language status, we should add something that allows to broadcast more general, less feature specific status, like [info] You are using version x of language y or [Error] Cannot validate because lib-foo is missing. That could look like

languages.pushErrorLanguageStatus('foo-lang', 'foo-lint missing');

fyi @mjbvz @kieferrm

[jrieken]

@egamma @dbaeumer I see your extensions (eslint, tslint) as a consumer of the second API, like languages.pushInformationStatus('javascipt', "Using ESLint 6.3")

[dbaeumer]

@jrieken I like this approach. There is one additional thing I would like to have: when clicking on the ESLint status icon it currently reveal the corresponding output channel which usually contains additional information. Will we be able to do the same.

And there is another feature I have: when you have errors in .eslintrc.json files I show the ESLint status icon even if no JS file is selected since technically the .eslinttc.json belongs to ESLint. And how would that work in the case that ESLint can validate HTML, vue, ... Would I register that for all languages ?

[jrieken]

@dbaeumer re #15454 (comment): It might be enough to allow for a DocumentSelector when passing on status, like so:

languages.pushInformationStatus(['javascript', { pattern: '**/.eslintrc.json' }], 'Some message')

[jrieken]

Still having thoughts on the API... Unsure if throwing/rejecting is proper enough despite its appeal for simplicity. These are the three options:

Option 1 - throwing/rejected promises

Providers can go the error-route (throw/Promise.reject) and signal their status using well-defined errors. This is inspired by HTTP status codes. It is stateless as each request (providerXYZ-call) is individually rejected.

// API:
export class Status {
  message: string;
  ...
}

// sample:
registerCompletionItemProvider('foo-lang', {
  provideCompletionItems(doc, pos, token) {
    if(somePreconditionNotMet) {
      throw new Status('short message', 'long message')
    }
  }
})

Pros:

• straight forward, often the provider methods is the place where things happens, e.g. foo-lang-lint not installed
• easy to scope to a feature/provider, like TypeScript IntelliSense
• stateless

Cons:

• sort of anti-API because the error types aren't defined anywhere as no function foo() throws XYZ-signatures are possible.
• not possible to signal status and provide a results (unless you create two providers which sort of defeats that API)
• needs to be repeated in every provider that is affected by a global'ish condition, like ATA and all providers that work on semantics
• operation failure (a real exception) and status can be confused easily

Option 2 - status objects knowing providers

A global function to create a provider-specific status object for which we can track updates etc. Allows to speak for multiple providers but is stateful

// API:
export interface ProviderStatus {
  unset(): void;
  set(message: string, more...)
}

export type Provider = CompletionItemProvider | DefinitionProvider | ...

export function createProviderStatus(provider: Provider, ...providers: Provider[]): LanguageStatus;

// sample:
const status = createProviderStatus(completionProvider, definitionProvider);

onDidChangeSomeStatus((message) => {
  if(message) {
    status.set('Fetching data for better IntelliSense')
  } else {
    status.unset();
  }
})

Pros:

• allows for status and results
• allows for generic, not provider specific status

Cons:

• harder to define to what scope a messages applies
• status message must be unset/has lifecycle

Option 3 - provider specific status

Each provider signals its status via a property.

// API:
export interface CompletionItemProvider {
  
  // signal provider status
  status: ProviderStatus;

  provideCompletionItems(document: TextDocument, position: Position, token: CancellationToken): ProviderResult<CompletionItem[] | CompletionList>;
  resolveCompletionItem?(item: CompletionItem, token: CancellationToken): ProviderResult<CompletionItem>;
}

// sample:
registerCompletionItemProvider('foo-lang', new class {

  status?: ProviderStatus;

  provideCompletionItems(doc, pos, token) {
    if(somePreconditionNotMet) {
      this.status = new ProviderStatus(someData);
    }
    // still possible to return a result
  }
})

Pros:

• allows for status and results
• allows to easily scope a message to a provider

Cons:

• fuzzy lifecycle - when do we read status? before and after calling the provide-function, should an event be fired when status changed?
• status must be unset/has lifecycle

[dbaeumer]

I like option 3 the best since it is easy to explain and local to the provider. However I don't see how this would help with stuff that a server or extension computes by itself like markers. There is no marker provider. So may be we need a combination of 2 & 3.