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

Range exceptioning on bad inputs #150

Open
sentry-io bot opened this issue Mar 31, 2022 · 5 comments · May be fixed by #584
Open

Range exceptioning on bad inputs #150

sentry-io bot opened this issue Mar 31, 2022 · 5 comments · May be fixed by #584
Labels
type:bug Something isn't working

Comments

@sentry-io
Copy link

sentry-io bot commented Mar 31, 2022

Sentry Issue: VSCODE-EXTENSION-1QN

Error: Range#create called with invalid arguments[[object Object], [object Object], undefined, undefined]
  File "/Users/pvu/.vscode/extensions/nomicfoundation.hardhat-solidity-0.2.0/server/out/index.js", line 31, in Object.Ki [as create]
    '{snip} s,Ri)};if(s.is(mi)&&s.is(Nn))return{start:mi,end:Nn};throw new Error("Range#create called with invalid arguments["+mi+", "+Nn+", "+gs+", "+R {snip}
  File "/Users/pvu/.vscode/extensions/nomicfoundation.hardhat-solidity-0.2.0/server/out/index.js", line 1722, in Mre
    '{snip} .character-r.length};return j20.TextEdit.replace(j20.Range.create(a,n),x)}function Mwx(x,n,r){let{rootPath:a,documentsAnalyzer:s}=r;return a {snip}
  File "/Users/pvu/.vscode/extensions/nomicfoundation.hardhat-solidity-0.2.0/server/out/index.js", line 1722, in <anonymous>
    '{snip} Analyzer:s})).map(f=>{let d=Vwx(f);return{label:d,textEdit:Mre(d,x,n),kind:M1.CompletionItemKind.Module,documentation:"Imports the package"} {snip}
  File "/Users/pvu/.vscode/extensions/nomicfoundation.hardhat-solidity-0.2.0/server/out/index.js", line 1722, in Mwx
    '{snip} mentsAnalyzer:s},n):jre({rootPath:a,documentsAnalyzer:s})).map(f=>{let d=Vwx(f);return{label:d,textEdit:Mre(d,x,n),kind:M1.CompletionItemKin {snip}
  File "/Users/pvu/.vscode/extensions/nomicfoundation.hardhat-solidity-0.2.0/server/out/index.js", line 1722, in Rre
    '{snip} wx(r);return f.concat(d)}else return Bwx(s)?Bre(x,s,u,n,a):Mwx(x,s,r)}function Bwx(x){return x.startsWith(".")}function Bre(x,n,r,a,s){if(/[ {snip}
...
(5 additional frame(s) were not displayed)

We are doing a range conversion that is throwing on bad inputs.

@github-actions
Copy link

This issue is also being tracked on Linear.

We use Linear to manage our development process, but we keep the conversations on Github.

LINEAR-ID: 6ff8371d-d1a0-4b34-82bf-bde8708d28be

@kanej kanej added the type:bug Something isn't working label Mar 31, 2022
@kanej kanej reopened this May 11, 2023
@kanej
Copy link
Member

kanej commented May 11, 2023

We are still seeing this in the sentry logs.

@OmarTawfik raised the point this may be related to some errors returned from solc not having source locations (i.e. error.sourceLocation). If that is the case we should deal errors of this sort as their own case, and eliminate the source of the bad inputs.

@OmarTawfik
Copy link
Contributor

OmarTawfik commented May 12, 2023

I have a suspicion it is related to checks like these:

if (!error.sourceLocation) {
this.logger.error(
new Error(
`Unattached error found: ${error.message} (${error.errorCode})`
)
);

if (error.sourceLocation === undefined) {
throw new Error("No source location");
}

I wonder if we should treat such locations as [0,0] instead of throwing/failing? Looks to me like it will still be an improvement to the end-user experience.

@kanej kanej removed their assignment May 16, 2023
@antico5
Copy link
Collaborator

antico5 commented May 18, 2023

@kanej This error is from 0.5.4 and earlier. Since then I've changed the import path completions logic and we don't have occurrences of this error on newer versions. Should I close the issue ?

@kanej
Copy link
Member

kanej commented May 22, 2023

@kanej This error is from 0.5.4 and earlier. Since then I've changed the import path completions logic and we don't have occurrences of this error on newer versions. Should I close the issue ?

Sentry is confusing on these sorts of thing. It doesn't necessarily merge new instances with the old version.

The Range#create exception is still happening: https://nomic-labs.sentry.io/issues/3758407495/?project=5469451&query=is%3Aunresolved+create&referrer=issue-stream&statsPeriod=14d&stream_index=0

kanej added a commit that referenced this issue Jul 9, 2024
If the import completion is triggered by deleting a quote on an existing
import and readding it then, the position is the start of the import but
we delete the entire length of the import line which creates a negative
position triggering the bug.

Instead, we short circuit. If the import is already the suggestion we
don't suggest it again.

Fixes #150.
@kanej kanej linked a pull request Jul 9, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
Status: In Review
Development

Successfully merging a pull request may close this issue.

4 participants