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

WB: RangeError invalid string length in invest stdErr callback #1092

Closed
davemfish opened this issue Oct 14, 2022 · 2 comments · Fixed by #1094
Closed

WB: RangeError invalid string length in invest stdErr callback #1092

davemfish opened this issue Oct 14, 2022 · 2 comments · Fixed by #1094
Assignees
Labels
bug Something isn't working in progress This issue is actively being worked on
Milestone

Comments

@davemfish
Copy link
Contributor

davemfish commented Oct 14, 2022

A user reported this error. It shows up as an uncaught exception in the main electron process. The user's logs are filled with an excessive amount of stderr from sdr. The following is logged over and over:

Traceback (most recent call last):
  File "src\natcap\invest\sdr\sdr_core.pyx", line 293, in natcap.invest.sdr.sdr_core._ManagedRaster._load_block
AttributeError: 'NoneType' object has no attribute 'astype'
Exception ignored in: 'natcap.invest.sdr.sdr_core._ManagedRaster.get'
Traceback (most recent call last):
  File "src\natcap\invest\sdr\sdr_core.pyx", line 293, in natcap.invest.sdr.sdr_core._ManagedRaster._load_block
AttributeError: 'NoneType' object has no attribute 'astype'

Since we are naively concatenating all invest stderr to a single string and then passing it to the browser when invest exits, we are apparently exceeding node's limit on string length nodejs/node#35973

Our stderr callback:

    const stdErrCallback = (data) => {
      logger.debug(`${data}`);
      investStdErr += `${data}`;
    };
    investRun.stderr.on('data', stdErrCallback);
@davemfish davemfish added the bug Something isn't working label Oct 14, 2022
@davemfish davemfish added this to the 3.12.1 milestone Oct 14, 2022
@davemfish davemfish self-assigned this Oct 14, 2022
@davemfish davemfish added the in progress This issue is actively being worked on label Oct 14, 2022
@davemfish
Copy link
Contributor Author

davemfish commented Oct 14, 2022

I think I'll bundle the fix for this into #1089

On second thought, I think the solution here is actually to simplify the design of the UI and probably remove the component that is receiving this stderr data. The error message is already clearly displayed & highlighted in red in the log. So we'll fix this separately from #1089.

@davemfish
Copy link
Contributor Author

It would also be nice to log these uncaught node exceptions. I think we can catch, log, and re-raise them with this listener https://nodejs.org/api/process.html#process_event_uncaughtexception

davemfish added a commit to davemfish/invest that referenced this issue Oct 17, 2022
davemfish added a commit to davemfish/invest that referenced this issue Oct 17, 2022
davemfish added a commit to davemfish/invest that referenced this issue Oct 17, 2022
davemfish added a commit to davemfish/invest that referenced this issue Oct 17, 2022
davemfish added a commit to davemfish/invest that referenced this issue Oct 17, 2022
davemfish added a commit to davemfish/invest that referenced this issue Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working in progress This issue is actively being worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant