Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ensure Terminating Thread error is correctly templatized #738

Merged
merged 3 commits into from
May 16, 2024

Conversation

darunrs
Copy link
Collaborator

@darunrs darunrs commented May 16, 2024

The error given to Stream Handler is occasionally a JSON object in disguise, and not an Error. As a result, calling error.toString() returns [Object object] rather than the error contents. I've added a check so that if the result of toString() is the earlier value, return a JSON.stringify result instead. Doing JSON.stringify on a proper Error type results in {}. The two cases must be handled separately.

To test this, I created test indexers which called one of the two pieces of code which throw unawaited async errors. I verified both wrote the error message and stack trace into the log table.

const timeoutPromise = new Promise((_, reject) => {
  setTimeout(() => {
      reject(new Error('Error thrown after 100ms'));
  }, 100);
});

context.db.IndexerStorage.upsert({}, [], []);

@darunrs darunrs requested a review from a team as a code owner May 16, 2024 00:30
@@ -67,8 +67,8 @@ export default class StreamHandler {
indexer.setStoppedStatus().catch((e) => {
this.logger.error('Failed to set stopped status for indexer', e);
});

const streamErrorLogEntry = LogEntry.systemError(`Encountered error processing stream: ${this.indexerConfig.redisStreamKey}, terminating thread\n${error.toString()}`, this.executorContext.block_height);
const errorAsString = error.toString() === '[object Object]' ? JSON.stringify(error) : error.toString();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const errorAsString = error.toString() === '[object Object]' ? JSON.stringify(error) : error.toString();
const errorMessage = error instanceof Error ? error.toString() : JSON.stringify(error);

I believe [Object object] would only appear if we are throwing an Object somewhere. I think we can just change this to the above to ensure we are only calling toString() on Errors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this and it seems to work. I think this is cleaner so I'll go with this. Thanks!

@darunrs darunrs merged commit 1eac90b into main May 16, 2024
3 checks passed
@darunrs darunrs deleted the fix-context-db-20240515 branch May 16, 2024 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants