Remove special errFn in plugin rest api. The statement that errFn must be synchronous is no longer correct after I89e3db3fb and I629739f6f . So this special re-throwing behaviour is not needed. Additionally instead of defining errFn here, we can simply reuse the throwingErrorCallback, that also ensures the error is thrown. (The only difference is formatting of the error message) Release-Notes: skip Change-Id: I05f7185d508bdb649616c5466d8734ade52df317
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts index e9c132a..d27bd2d 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts
@@ -9,22 +9,16 @@ import {PluginApi} from '../../../api/plugin'; import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api'; import {ReportingService} from '../../../services/gr-reporting/gr-reporting'; -import {readJSONResponsePayload} from '../gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper'; +import { + readJSONResponsePayload, + throwingErrorCallback, +} from '../gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper'; async function getErrorMessage(response: Response): Promise<string> { const text = await response.text(); return text || `${response.status}`; } -// This is an internal error, that must never be visible outside of this -// file. It is used only inside GrPluginRestApi.send method. See detailed -// explanation in the GrPluginRestApi.send method. -class ResponseError extends Error { - public constructor(readonly response: Response) { - super(); - } -} - export class GrPluginRestApi implements RestPluginApi { constructor( private readonly restApi: RestApiService, @@ -120,53 +114,31 @@ contentType?: string ) { this.reporting.trackApi(this.plugin, 'rest', 'send'); - // Plugins typically don't want Gerrit to show error dialogs for failed - // requests. So we are defining a default errFn here, even if it is not - // explicitly set by the caller. - // TODO: We are soon getting rid of the `errFn` altogether. There are only - // 2 known usages of errFn in plugins: delete-project and verify-status. - errFn = - errFn ?? - ((response: Response | null | undefined, error?: Error) => { - if (error) throw error; - // Some plugins show an error message if send is failed, smth like: - // pluginApi.send(...).catch(err => showError(err)); - // The response can contain an error text, but getting this text is - // an asynchronous operation. At the same time, the errFn must be a - // synchronous function. - // As a workaround, we throw an ResponseError here and then catch - // it inside a catch block below and read the message. - if (response) throw new ResponseError(response); - throw new Error('Generic REST API error.'); - }); - return this.fetch(method, url, payload, errFn, contentType) - .then(response => { - // Will typically not happen. The response can only be unset, if the - // errFn handles the error and then returns void or undefined or null. - // But the errFn above always throws. - if (!response) { - throw new Error('plugin rest-api call failed'); - } - // Will typically not happen. errFn will have dealt with that and the - // caller will get a rejected promise already. - if (response.status < 200 || response.status >= 300) { - return getErrorMessage(response).then(msg => - Promise.reject(new Error(msg)) - ); - } else { - return readJSONResponsePayload(response).then( - obj => obj.parsed - ) as Promise<T>; - } - }) - .catch(err => { - if (err instanceof ResponseError) { - return getErrorMessage(err.response).then(msg => { - throw new Error(msg); - }); - } - throw err; - }); + return this.fetch( + method, + url, + payload, + errFn ?? throwingErrorCallback, + contentType + ).then(response => { + // Will typically not happen. The response can only be unset, if the + // errFn handles the error and then returns void or undefined or null. + // But the errFn above always throws. + if (!response) { + throw new Error('plugin rest-api call failed'); + } + // Will typically not happen. errFn will have dealt with that and the + // caller will get a rejected promise already. + if (response.status < 200 || response.status >= 300) { + return getErrorMessage(response).then(msg => + Promise.reject(new Error(msg)) + ); + } else { + return readJSONResponsePayload(response).then( + obj => obj.parsed + ) as Promise<T>; + } + }); } get<T>(url: string) {