Skip to content

Commit

Permalink
Do not show error on messages from Forwarder in show single message p…
Browse files Browse the repository at this point in the history
…age (#12485) (#13386)

* Extract checking for local node from hook

* Refactor useMessage effect with async function

This will make the code a bit easier to read when changing logic to
check local node.

* Only load local inputs on show message page

On the show single message page, use `isLocalNode` function to decide
whether the message was received in a local node directly or through a
Forwarder. This way we can avoid failures when trying to fetch an input
from a local node, that actually belongs to a Forwarder.

Fixes https://github.com/Graylog2/graylog-plugin-enterprise/issues/3472

Co-authored-by: Edmundo Alvarez <edmundo@graylog.com>
  • Loading branch information
mnin and Edmundo Alvarez committed Sep 15, 2022
1 parent 10a5396 commit d5832cf
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 19 deletions.
26 changes: 24 additions & 2 deletions graylog2-web-interface/src/pages/ShowMessagePage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ const mockLoadMessage = jest.fn();
const mockGetInput = jest.fn();
const mockListNodes = jest.fn();
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const mockListStreams = jest.fn((...args) => Promise.resolve([]));
const mockListStreams = jest.fn((..._args) => Promise.resolve([]));
const mockPluginStoreExports = jest.fn();

jest.mock('stores/nodes/NodesStore', () => ({
NodesActions: { list: (...args) => mockListNodes(...args) },
Expand All @@ -53,6 +54,12 @@ jest.mock('stores/streams/StreamsStore', () => ({ listStreams: (...args) => mock
jest.mock('views/logic/fieldtypes/useFieldTypes');
jest.mock('routing/withParams', () => (x) => x);

jest.mock('graylog-web-plugin/plugin', () => ({
PluginStore: {
exports: jest.fn((...args) => mockPluginStoreExports(...args)),
},
}));

type SimpleShowMessagePageProps = {
index: string,
messageId: string,
Expand All @@ -65,7 +72,7 @@ const SimpleShowMessagePage = ({ index, messageId }: SimpleShowMessagePageProps)

describe('ShowMessagePage', () => {
beforeEach(() => {
asMock(useFieldTypes).mockClear();
jest.clearAllMocks();
asMock(useFieldTypes).mockReturnValue({ data: [] });
});

Expand Down Expand Up @@ -114,4 +121,19 @@ describe('ShowMessagePage', () => {

expect(container).toMatchSnapshot();
});

it('does not fetch input when opening message from forwarder', async () => {
mockPluginStoreExports.mockReturnValue([{
isLocalNode: jest.fn(() => false),
}]);

mockLoadMessage.mockImplementation(() => Promise.resolve(message));
mockGetInput.mockImplementation(() => Promise.resolve());
mockListStreams.mockImplementation(() => Promise.resolve([]));

render(<SimpleShowMessagePage index="graylog_5" messageId="20f683d2-a874-11e9-8a11-0242ac130004" />);
await screen.findByText(/Deprecated field/);

expect(mockGetInput).not.toHaveBeenCalled();
});
});
18 changes: 11 additions & 7 deletions graylog2-web-interface/src/pages/ShowMessagePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import StreamsStore from 'stores/streams/StreamsStore';
import { InputsActions } from 'stores/inputs/InputsStore';
import { MessagesActions } from 'stores/messages/MessagesStore';
import { NodesActions } from 'stores/nodes/NodesStore';
import { isLocalNode } from 'views/hooks/useIsLocalNode';

type Props = {
params: {
Expand Down Expand Up @@ -65,19 +66,22 @@ const useMessage = (index: string, messageId: string) => {
const [inputs, setInputs] = useState<Immutable.Map<string, Input>>(Immutable.Map());

useEffect(() => {
MessagesActions.loadMessage(index, messageId)
.then((_message) => {
setMessage(_message);
const fetchData = async () => {
const _message = await MessagesActions.loadMessage(index, messageId);
setMessage(_message);

if (_message.source_input_id && await isLocalNode(_message.fields.gl2_source_node)) {
const input = await InputsActions.get(_message.source_input_id);

return _message.source_input_id ? InputsActions.get(_message.source_input_id) : Promise.resolve();
})
.then((input) => {
if (input) {
const newInputs = Immutable.Map({ [input.id]: input });

setInputs(newInputs);
}
});
}
};

fetchData();
}, [index, messageId, setMessage, setInputs]);

return { message, inputs };
Expand Down
33 changes: 23 additions & 10 deletions graylog2-web-interface/src/views/hooks/useIsLocalNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,37 @@
* along with this program. If not, see
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
import { PluginStore } from 'graylog-web-plugin/plugin';
import { useState, useEffect } from 'react';

import usePluginEntities from 'views/logic/usePluginEntities';

const useIsLocalNode = (nodeId: string) => {
const forwarderPlugin = usePluginEntities('forwarder');
export const isLocalNode = async (nodeId: string) => {
const forwarderPlugin = PluginStore.exports('forwarder');
const _isLocalNode = forwarderPlugin?.[0]?.isLocalNode;
const [isLocalNode, setIsLocalNode] = useState<boolean | undefined>();

useEffect(() => {
try {
if (nodeId && _isLocalNode) {
_isLocalNode(nodeId).then(setIsLocalNode, () => setIsLocalNode(true));
} else {
setIsLocalNode(true);
return _isLocalNode(nodeId);
}
} catch (e) {
// Do nothing
}

return true;
};

const useIsLocalNode = (nodeId: string) => {
const [_isLocalNode, setIsLocalNode] = useState<boolean | undefined>();

useEffect(() => {
const checkIsLocalNode = async () => {
const result = await isLocalNode(nodeId);
setIsLocalNode(result);
};

checkIsLocalNode().catch(() => setIsLocalNode(true));
}, [nodeId, _isLocalNode]);

return { isLocalNode };
return { isLocalNode: _isLocalNode };
};

export default useIsLocalNode;

0 comments on commit d5832cf

Please sign in to comment.