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

Make auto-fixing of stack traces optional when using ssrLoadModule #7046

Closed
4 tasks done
Rich-Harris opened this issue Feb 22, 2022 · 4 comments · Fixed by #7048
Closed
4 tasks done

Make auto-fixing of stack traces optional when using ssrLoadModule #7046

Rich-Harris opened this issue Feb 22, 2022 · 4 comments · Fixed by #7048
Labels
feat: ssr p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)

Comments

@Rich-Harris
Copy link
Contributor

Clear and concise description of the problem

When an error occurs during SSR, the stack trace needs fixing in order to be correct:

try {
  result = mod.foo();
} catch (error) {
  vite.ssrFixStacktrace(error);
  handle_error(error);
}

But if the error occurred during ssrLoadModule, Vite will fix the stack trace before the error is thrown. As discussed in #7045, this results in a broken sourcemap at best and a cryptic "TypeError: Line must be greater than or equal to 1, got -1" error at worst:

try {
+ const mod = await vite.ssrLoadModule('my-module.js');
  result = mod.foo();
} catch (error) {
+ // yikes, we don't know if the stack trace was already fixed or not
- vite.ssrFixStacktrace(error);
  handle_error(error);
}

It's possible to work around it...

let mod;

try {
  mod = await vite.ssrLoadModule('my-module.js');
} catch (error) {
  handle_error(error);
}

try {
  result = mod.foo();
} catch (error) {
  vite.ssrFixStacktrace(error);
  handle_error(error);
}

...but when module loads are happening several function call boundaries away from the try/catch, this becomes problematic — all you can do is remove ssrFixStacktrace from the top-level catch and wrap everything except calls to ssrLoadModule with more granular try/catch blocks and do ssrFixStacktrace in those instead. It can get out of hand quite rapidly.

Suggested solution

For me, the ideal would be if ssrLoadModule didn't fix stack traces at all (or log errors, for that matter) — there's nothing particularly special about errors that occur during module load rather than inside a function.

Since that would be a breaking change, it could be an config option instead:

// vite.config.js, or inline config
const config = {
  ssr: {
    preserveStacktrace: true
  }
};

Alternative

Instead of putting the option in the config, it could be an option to ssrLoadModule itself. The docs suggest that ssrLoadModule already takes an options object...

image

...though I think that's incorrect?

ssrLoadModule(url) {
server._ssrExternals ||= resolveSSRExternal(
config,
server._optimizeDepsMetadata
? Object.keys(server._optimizeDepsMetadata.optimized)
: []
)
return ssrLoadModule(url, server)
},
Regardless, an option like preserveStacktrace: true could be a solution here.

Additional context

No response

Validations

@Rich-Harris
Copy link
Contributor Author

How so? They don't appear at all related to me

@benmccann
Copy link
Collaborator

Here's the code that does it (it doesn't call ssrFixStacktrace, but invokes the same two methods):

const stacktrace = ssrRewriteStacktrace(e.stack, moduleGraph)

@Niputi
Copy link
Contributor

Niputi commented Feb 22, 2022

sorry. after reading it a second time now I see it's not related.
first time I read this, it somehow felt similar in my head

@benmccann
Copy link
Collaborator

I sent a PR for this: #7048

@benmccann benmccann added p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) and removed enhancement: pending triage labels Feb 22, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: ssr p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants