Skip to content

Commit

Permalink
Fix #859 Improve the way to handle authorize fn errors in built-in re…
Browse files Browse the repository at this point in the history
…ceivers
  • Loading branch information
seratch committed Aug 26, 2021
1 parent bcca0ee commit 11f7c9e
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 4 deletions.
6 changes: 3 additions & 3 deletions src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import {
RespondArguments,
} from './types';
import { IncomingEventType, getTypeAndConversation, assertNever } from './helpers';
import { CodedError, asCodedError, AppInitializationError, MultipleListenerError } from './errors';
import { CodedError, asCodedError, AppInitializationError, MultipleListenerError, ErrorCode } from './errors';
// eslint-disable-next-line import/order
import allSettled = require('promise.allsettled'); // eslint-disable-line @typescript-eslint/no-require-imports
// eslint-disable-next-line @typescript-eslint/no-require-imports
Expand Down Expand Up @@ -677,7 +677,7 @@ export default class App {
}
} catch (error) {
this.logger.warn('Authorization of incoming event did not succeed. No listeners will be called.');
error.code = 'slack_bolt_authorization_error';
error.code = ErrorCode.AuthorizationError;
return this.handleError(error);
}

Expand Down Expand Up @@ -874,7 +874,7 @@ const tokenUsage =
'should be initialized with oauth installer or authorize.';

function defaultErrorHandler(logger: Logger): ErrorHandler {
return (error) => {
return (error: CodedError) => {
logger.error(error);

return Promise.reject(error);
Expand Down
18 changes: 17 additions & 1 deletion src/receivers/ExpressReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@ import tsscmp from 'tsscmp';
import { Logger, ConsoleLogger, LogLevel } from '@slack/logger';
import { InstallProvider, CallbackOptions, InstallProviderOptions, InstallURLOptions } from '@slack/oauth';
import App from '../App';
import { ReceiverAuthenticityError, ReceiverMultipleAckError, ReceiverInconsistentStateError } from '../errors';
import {
ReceiverAuthenticityError,
ReceiverMultipleAckError,
ReceiverInconsistentStateError,
ErrorCode,
CodedError,
} from '../errors';
import { AnyMiddlewareArgs, Receiver, ReceiverEvent } from '../types';
import { renderHtmlForInstallPath } from './render-html-for-install-path';

Expand Down Expand Up @@ -213,6 +219,16 @@ export default class ExpressReceiver implements Receiver {
this.logger.debug('stored response sent');
}
} catch (err) {
if ('code' in err) {
// CodedError has code: string
const errorCode = (err as CodedError).code;
if (errorCode === ErrorCode.AuthorizationError) {
// authorize function threw an exception, which means there is no valid installation data
res.status(401).send();
isAcknowledged = true;
return;
}
}
res.status(500).send();
throw err;
}
Expand Down
12 changes: 12 additions & 0 deletions src/receivers/HTTPReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
ReceiverInconsistentStateError,
HTTPReceiverDeferredRequestError,
ErrorCode,
CodedError,
} from '../errors';

export interface HTTPReceiverOptions {
Expand Down Expand Up @@ -370,6 +371,17 @@ export default class HTTPReceiver implements Receiver {
this.logger.debug('stored response sent');
}
} catch (err) {
if ('code' in err) {
// CodedError has code: string
const errorCode = (err as CodedError).code;
if (errorCode === ErrorCode.AuthorizationError) {
// authorize function threw an exception, which means there is no valid installation data
res.writeHead(401);
res.end();
isAcknowledged = true;
return;
}
}
this.logger.error('An unhandled error occurred while Bolt processed an event');
this.logger.debug(`Error details: ${err}, storedResponse: ${storedResponse}`);
res.writeHead(500);
Expand Down

0 comments on commit 11f7c9e

Please sign in to comment.