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

[Feature]: Support none standard errors - "Cannot set property message of which has only a getter" #411

Closed
aradbar opened this issue Jul 11, 2023 · 7 comments · Fixed by #414

Comments

@aradbar
Copy link
Contributor

aradbar commented Jul 11, 2023

Describe the bug

This is a follow up to this fix - #405

The issue was getting the error -
Error occurred during processing of an SQS message - Cannot set property message of which has only a getter
From of the line -
err.message = `Unexpected message handler failure: ${err.message}`;

At the time we thought the problem was that err is not an Error instance so the fix deals with that case, but my team kept seeing this issue. We dug a little deeper and found that some libraries have errors that inherit from Error but override message to be readonly. The example we encountered is the error DOMException from jsdom

I also see that this was raised before regarding a different error #402 where you said it wasn't really a bug in sqs-consumer.

I think you should reconsider it a bug that needs to be addressed by sqs-consumer code, since you're assuming every Error will have a writable message but this is not always the case. Also there's no workaround for me as a user of sqs-consumer because I can't override the error handler.

Your minimal, reproducible example

Steps to reproduce

  1. pass any error with a readonly message to the error handler

Expected behavior

  1. error handler should through the original error

How often does this bug happen?

None

Screenshots or Videos

No response

Platform

v16.15.1

Package version

v7.2.0

AWS SDK version

No response

Additional context

No response

@nicholasgriffintn nicholasgriffintn added feature-request feature request and removed bug labels Jul 11, 2023
@nicholasgriffintn nicholasgriffintn changed the title [Bug]: Cannot set property message of which has only a getter [Feature]: Support none standard errors - "Cannot set property message of which has only a getter" Jul 11, 2023
@nicholasgriffintn
Copy link
Member

Yup, this still isn't a bug, a bug is required to be related to expected and documented functionality, we do not specifically support this implementation.

Will re label, happy to take a PR on this, but outside of that, it's not really our focus.

@utluiz
Copy link

utluiz commented Aug 5, 2023

A possible workaround for this scenario would be simply wrapping the error i the handler before throwing it to the consumer. Pseudo-code:

const handler = async (message) => {
  try {
    // business logic + validation
  } catch (err: unknown) {
    throw new Error('My error message', { cause: err })
  }
}

That way the non-standard error would be wrapped in a standard error.

@github-actions
Copy link

github-actions bot commented Sep 5, 2023

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Sep 5, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 5 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 10, 2023
@mhuggins
Copy link

I ran into this issue today too (same exact issue as #402, as I'm using zod to validate message structure as well). I created a helper function to wrap my handleMessage function, which might be of use to others.

import { ConsumerOptions } from 'sqs-consumer';

export type HandleMessage = NonNullable<ConsumerOptions['handleMessage']>;

const getErrorMessage = (err: unknown) => {
  if (typeof err === 'string') {
    return err;
  }
  if (err instanceof Error) {
    return err.message;
  }
  return 'Unknown Error';
};

const wrapErrors = (messageHandler: HandleMessage): HandleMessage => async (...args) => {
  try {
    const result = await messageHandler(...args);
    return result;
  } catch (err) {
    throw new Error(getErrorMessage(err), { cause: err });
  }
};

Now my consumer can wrap the any handler passed into handleMessage. (Note that processMessage is my custom handler, which I didn't include here, but it is also of type HandleMessage that I typed in the code snippet above.)

import { Consumer, ConsumerOptions } from 'sqs-consumer';
import { processMessage } from './processMessage';
import { wrapErrors } from './wrapErrors';

export const consumer = Consumer.create({
  handleMessage: wrapErrors(processMessage),
  // ...snip...
});

@nicholasgriffintn
Copy link
Member

This shouldn't have been closed.

Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants